Add _precomputable_kwarg_names to BaseExtractor

[chrishalcrow]

As discussed in #3685 (comment) and in maintainer meetings, it would be handy to have a _precomputable_kwargs in the BaseExtractor, especially for Preprocessors.

These are kwargs which can be precomputed for use in a preprocessor. For instance, if you know the whitening matrices, you can pass these directly instead of computing them. If you know the bad channel ids, you can pass these to RemoveBadChannels (once it exists #3685) and if you know your motion object, you could pass this to a CorrectMotion class (once this exists too!).

This can also be used when saving preprocessed recordings. If we save the provenance and the precomputed data, we can load a preprocessed recording much faster. This will be implemented in a later PR.

This PR has no functionality so there are no new tests.

Questions:

• Naming. @alejoe91 had suggested on _precomputed_kwargs but I think this is confusing since these kwargs aren't always precomputed, it's just that they can be precomputed. So I've gone for _precomputable_kwargs. Opinions?
• Should this be in BaseExtractor? Could be in BasePreprocessor? Any use cases for it in scenarios other than preprocessing?

[alejoe91]

Thanks @chrishalcrow

I think that BaseExtractor is ok!

[alejoe91]

@samuelgarcia ok to you?

[zm711]

agree able is better if its possible but not necessarily done :)

[chrishalcrow]

@samuelgarcia _precomputable_kwargs is now a class attribute.

[h-mayorquin]

My two cents:

1. Kwargs is usually used (in this codebase as well) for dict objects. I think ._precomputable_kwargs is a good name but it should be a dict and follow kwarg conventions.
2. I say isolate to BasePocessor and then generalize when and if we needed. It is good to keep complexity away from the core as long as we can.

I can't really say a lot because I have not seen it in use. Perhaps a use case other than #3781 is the estimated noise and std of recordings. We already have some mechanism to treat that as special quantities and maybe they should be unified with this.Also, how will this work with the json serialization is not clear to me. Maybe you discussed this on the meeting that I missed.

All of that said, these is a private attribute so I am not too concerned if we need to change it later.

[alejoe91]

My two cents:

1. Kwargs is usually used (in this codebase as well) for dict objects. I think ._precomputable_kwargs is a good name but it should be a dict and follow kwarg conventions.
2. I say isolate to BasePocessor and then generalize when and if we needed. It is good to keep complexity away from the core as long as we can.

I can't really say a lot because I have not seen it in use. Perhaps a use case other than #3781 is the estimated noise and std of recordings. We already have some mechanism to treat that as special quantities and maybe they should be unified with this.Also, how will this work with the json serialization is not clear to me. Maybe you discussed this on the meeting that I missed.

All of that said, these is a private attribute so I am not too concerned if we need to change it later.

1. What about _precomputable_kwarg_names?

2. I vote for keeping it in base, because soon we will need some logic in the loading functions to either keep or discard precomputable kwargs.

[chrishalcrow]

When we discussed, we agreed not to use this to store any kwargs themselves, just the information about which kwargs could be precomputed. So I think a list is the correct structure; very happy to call _precomputable_kwarg_names.

I'm indifferent to putting it in BaseExtractor or BaseProcessor.

[alejoe91]

When we discussed, we agreed not to use this to store any kwargs themselves, just the information about which kwargs could be precomputed. So I think a list is the correct structure; very happy to call _precomputable_kwarg_names.

I'm indifferent to putting it in BaseExtractor or BaseProcessor.

Yes, but what @h-mayorquin is saying is that the name _precomputable_kwargs suggests that it contains kwargs (a dict).

[h-mayorquin]

OK, we will see how it is used.

It could also be kwargs_keys to keep with the dict semantics but names is just fine if not more expressive.

[samuelgarcia]

super