-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
_feature as log attribute in LibrosaFeatureExtractor? #427
Comments
I think the original idea was that users would initialize the subclasses rather than the base class, in which case it wouldn't matter (you'd always have the class name to go by). But since we don't actually prevent instantiation of the base class (and I see no reason to do that), I agree that this is problematic. If we want to add |
Re: the general question of whether to have a single class or multiple classes for wrapping librosa, IIRC we went back and forth on that, and ultimately decided with the current explicit approach. But we could always revisit that decision if, e.g., librosa keeps adding new features, and we don't want to have to keep maintaining a whole hierarchy of wrappers. [EDIT: on further reflection, I think the main reason for going with the present approach was that some extractors need a bit of custom code to deal with the outputs, which is still the case.] |
okay let's maybe leave this open for now, not super high-priority and we can maybe come back to it later on. |
Currently, the
feature
argument for LibrosaFeatureExtractor (which selects the target librosa feature) is not stored inlog_attributes
.It may make sense to do so, as it is otherwise not possible to initialize multiple extractors for different librosa features if
cache_transformers
is set toTrue
.The text was updated successfully, but these errors were encountered: