pliers icon indicating copy to clipboard operation
pliers copied to clipboard

Explicit __init__ methods for Transformer classes

Open tyarkoni opened this issue 7 years ago • 2 comments

A minor issue with the current Transformer hierarchy is that most of the user-facing classes don't have to define explicit __init__ methods. This isn't a problem functionally, but it makes it hard for users to figure out what the initialization arguments are--and this can be quite important (e.g., in the case of arguments like api_key). Even though it's redundant, we should probably insist on every public-facing Transformer class explicitly implementing __init__ with all the main arguments visible, even if all the method does is turn right around and pass everything up to the superclass.

tyarkoni avatar Dec 29 '17 19:12 tyarkoni

I agree with this issue as well, it just will be tedious to do everything. This also leaves us open to interpreting what "main arguments" are, but I suspect we'll just have to use our judgment per transformer. Do the LibrosaFeatureExtractors look good for this pattern?

qmac avatar Jan 04 '18 00:01 qmac

Yeah I think we probably need to do the librosa Extractors this way too. But there's no rush on this--it's not a high priority, and we can chip away at it as time allows. It was mainly salient to me because I'm close to pushing a big PR that adds Sphinx docs, and the autodocs don't help very much when the args are basically just **kwargs.

tyarkoni avatar Jan 04 '18 00:01 tyarkoni