Proposal for auto import extensions module.

[samuelgarcia]

Hi @zm711 @alejoe91.
This is proposal to auto load extensions : when extension is builin and not registered because not impotred yet : then we have an auto load mechanism. This should help the end user.
Tell me what you think.

This is hard to be tested in core tests because it auload submodules which will fail on core testing because of dependencies.
So I made local tests and it works.

This should fix partially #2560.

[alejoe91]

Suggested change

print(f"module '{module}' is imported automatically for extension '{extension_name}'")

should we remove the print?

[samuelgarcia]

I think it is good to have a message for auto loading no ?
Maybe a warning

[alejoe91]

not sure

[zm711]

I'm torn here... If someone requests this functionality they should expect us to import what is needed to do the operation right? I guess the concern would be some sort of namespace clash? Like if we run an extractor the user expects a read operation from a file, we don't explicitly say we are creating a memmap which may lead to RAM exhaustion. On the other hand people complain about things failing silently for the job_kwargs.

[alejoe91]

The extractors example is a very good point. If someone saves a JSON file with dump_to_json, all the modules from extractors, preprocessing, etc. will be loaded.
In that case, we store the path to the class so imports are minimal.

Maybe we could do the same here? e.g.

_builtin_extensions = {
    "spike_amplitudes": "spikeinterface.postprocessing.spike_amplitudes.SpikeAmplitudeExtension",
    ...
}

[h-mayorquin]

I think it should be removed. What is the user supposed to do with this information?

This sounds more like logging and when it is output by default the convention is to print to standard error as far as I am aware. So a warning. But I personally think that warnings should be actionable so not even sure about that one.

[zm711]

As long as you're okay with autoload then this seems good to me. I can also test locally this afternoon. Otherwise, I think just the additional error messaging is much improved.

[zm711]

I'm torn here... If someone requests this functionality they should expect us to import what is needed to do the operation right? I guess the concern would be some sort of namespace clash? Like if we run an extractor the user expects a read operation from a file, we don't explicitly say we are creating a memmap which may lead to RAM exhaustion. On the other hand people complain about things failing silently for the job_kwargs.

[zm711]

So testing this isn't quite working locally for me. I have a binary_folder with following extensions.

upon loading I get:

and if I try to grab an extension I get

I believe there are a few issues:

get_extension makes a call to has_extension which I believe is failing.
And during loading we also make all call to get_saved_extension_names which relies on _possible_extensions which I believe is not being populated.

What else do you want me to try locally Sam? Were you using zarr backend for this or did you get this to work with binary?

[zm711]

Also I never get to the print statement we were discussing above (L1082).

[samuelgarcia]

Oups. My bad I tested the compute() let me finish this correctly.

[zm711]

Happy to test compute() tomorrow (that was actually the one thing I didn't test-haha). But yeah for me I think load is a nicer target since I always import everything when I'm doing computations, but when I'm reloading sometimes I just do core to check things so it might miss some of the extensions.

[zm711]

Tested both compute and load_sorting_analyzer and both worked for me locally.