flatironinstitute / CaImAn

Computational toolbox for large scale Calcium Imaging Analysis, including movie handling, motion correction, source extraction, spike deconvolution and result visualization.
https://caiman.readthedocs.io
GNU General Public License v2.0
639 stars 370 forks source link

Remove pylab imports #1302

Closed EricThomson closed 7 months ago

EricThomson commented 8 months ago

Description

Will close out item on Roadmap (#1142 ).

Pylab is a relic of the days when Python was trying to be like Matlab and its use is strongly discouraged even in its source code. This gets rid of it.

It includes things like:

from numpy import *
from numpy.fft import *
from numpy.random import *
from numpy.linalg import *

And it makes Caiman less readable with pl.foo() syntax whereas people expect plt.foo() when using the recommended api.

Type of change

I'll be testing out things as I go through the different modules.

One question: should we change the name of the back end in movies.py from pylab to matplotlib? It seems yes. And, should we warn people?

My guess is nobody uses it, I have tested it (it works): the quality is horrible, we never recommend it -- in fact we literally throw a warning when people even try to use it. My inclination is just to make this change in the name of the back end. But if others thing we should give warnings at first, and change the name in a couple of cycles, I'd listen.

(Aside: I think a more important question is whether we even want this matplotlib interface for movies in there in the first place, but that's a bigger issue outside the scope of this PR. It seems like a maintenance burden not worth having.)

EricThomson commented 8 months ago

I believe that's everything: I changed the backend keyword from pylab to pyplot (using matplotlib seemed weird and this seems closer to the original, and will discourage its use hopefully -- I thought using matplotlib would make people want to use it: I do think it will be worth considering deprecating this option at some point in the future).

I ran through demos and they seemed fine. caimanmanager tests ran fine.