search

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
640 stars 370 forks source link

fix mat support #1337

Closed pgunn closed 7 months ago

pgunn commented 7 months ago

This fixes use of scipy APIs that are no longer in scipy to read old-style .mat files. ( #1336 )

Support for the newer hdf5-based mat format is retained (and by newer we're talking about a change made a long time ago).

There were two other ways to fix the import problem: 1) Have a slightly friendlier user message for people trying to load old mat files, by doing try/except in a few places, catching the exception, and guessing that they were trying to load the earlier format and tell them. 2) Attempt to use scipy.io.matlab.loadmat() instead, building support for that back into the codebase (classic mat handling had been broken for awhile and would've needed some work)

I decided not to do the first because it complicates some already complicated code for the sake of an obsolete format (and the try/exception model actually wouldn't tell us for sure why an OSError was thrown so it could be confusing no matter what). I decided not to do the second because it's a bit of work for a format people shouldn't be using anymore and it's something we may have decided to stop supporting anyhow even if the code was working (unless users told us they need it).

If some users of old-style mat files turn up and tell us they really need the old format (even though it hasn't worked in caiman for several releases at least), I can move to the second solution, but I'm otherwise happy with this choice.

pgunn commented 7 months ago

Ugh, I based this off the wrong branch. Redoing the diff.

ethanbb commented 7 months ago

I would mildly favor at least catching whatever error gets thrown for older .mat files and erroring with a message that suggests that as a possible cause of the error if the extension is .mat, given how like I said the older format is still the default. But I do support dropping any appearance of real support for it.

pgunn commented 7 months ago

Hm. Okay. The code for that isn't too bad/bulky.

ethanbb commented 7 months ago

Thanks! Yeah, the days of intentionally saving non-HDF5 mat files are long gone (for everyone I know), but the days of accidentally ending up with them are unfortunately still with us.