PsychoinformaticsLab
_feature as log attribute in LibrosaFeatureExtractor?
Currently, the feature argument for LibrosaFeatureExtractor (which selects the target librosa feature) is not stored in log_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 to True.
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 _feature to logging, probably best to rename that to feature_name throughout, for clarity. Feel free to open a PR.
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.
For reference, the usage case here was the attempt to pass different librosa extractors to a graph in a list created via list comprehension (e.g. [LibrosaFeatureExtractor(feature=f) for f in feature_list]). Being able to do so is convenient in this case, but not so crucial