Fix use of dataclasses for python 3.11

simon-ball commented 9 months ago

Suite2p is currently non-functional in Python 3.11 due to a change in the way the built-in library dataclasses handles default values: https://docs.python.org/3.11/whatsnew/3.11.html#dataclasses (cross ref #1029 )

This change modifies the default creation in two ways to address the limitation:

rather than directly assigning a (mutable) default, a default is generated independently in each instance by defining a default_factory instead
default_factory expects a function that will be called on ROI.__init__(), so move default generation to a lambda expression to be called as and when needed. Depending on style preferences, this could potentially be integrated with the suite2p.detection.distance_kernel method directly, since as far as I can tell the default in the ROI dataclass is the sole purpose of that function.

The new method is back-compatible at least as far as Python 3.8.

I have not tested for any other 3.11 incompatibilities yet

landoskape commented 8 months ago

What issues in suite2p does this fix? Or does it generally not work at all with 3.11?

simon-ball commented 8 months ago

Suite2p is currently non-functional in Python 3.11, as in, it cannot be successfully imported. At all.

$ conda create -n test311 python=3.11 -y
$ conda activate test311
$ pip install suite2p
$ python -c "import suite2p"

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/__init__.py", line 6, in <module>
    from .run_s2p import run_s2p, run_plane, pipeline
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/run_s2p.py", line 15, in <module>
    from . import extraction, io, registration, detection, classification, default_ops
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/extraction/__init__.py", line 5, in <module>
    from .extract import create_masks_and_extract, enhanced_mean_image, extract_traces_from_masks, extraction_wrapper
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/extraction/extract.py", line 11, in <module>
    from .masks import create_masks
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/extraction/masks.py", line 9, in <module>
    from ..detection.sparsedetect import extendROI
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/detection/__init__.py", line 4, in <module>
    from .detect import detect, detection_wrapper, bin_movie
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/detection/detect.py", line 9, in <module>
    from . import sourcery, sparsedetect, chan2detect, utils
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/detection/sourcery.py", line 12, in <module>
    from .stats import fitMVGaus
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/site-packages/suite2p/detection/stats.py", line 52, in <module>
    @dataclass(frozen=True)
     ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/dataclasses.py", line 1220, in wrap
    return _process_class(cls, init, repr, eq, order, unsafe_hash,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/dataclasses.py", line 958, in _process_class
    cls_fields.append(_get_field(cls, name, type, kw_only))
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/miniconda3/envs/imtest311/lib/python3.11/dataclasses.py", line 815, in _get_field
    raise ValueError(f'mutable default {type(f.default)} for field '
ValueError: mutable default <class 'numpy.ndarray'> for field rsort is not allowed: use default_factory

The proximate cause of that inability to import appears to be that in 3.10 and earlier, the built-in library dataclasses allowed the default value of a class decorated with @dataclasses.dataclass to be mutable, but that in Python 3.11, this became forbidden (prior link from above: https://docs.python.org/3.11/whatsnew/3.11.html#dataclasses). As I understand it, the reason for doing so is to ensure that you cannot change the default value in all extant instances of that dataclass by overwriting it in one instance.

As the exception generated on importing explains, the object suite2p.detection.stats.ROI has a field rsort that is assigned a mutable default value, i.e. a NumPy array.

This PR attempts to address that problem as described above. In terms of the apparent goal of the change to dataclasses in 3.11, the shared mutable default value for ROI.rsort is made un-shared by instead assigning a generator/factory that will be called in ROI.__init__, resulting in identical but independent copies of the same default value.

simon-ball commented 8 months ago

This is, I assume the problem that the change to dataclasses in Python 3.11 is supposed to solve:

from suite2p.detection.stats import ROI
import numpy as np
z1 = np.zeros((2,2))
z2 = np.zeros((3,3))
a = ROI(z1,z1,z1,z1,False)
b = ROI(z2,z2,z2,z2,False)
a.rsort is b.rsort
>>> True
a.rsort
>>> array([ 0.        ,  1.        ,  1.        , ..., 42.42640687, 42.42640687, 42.42640687])
a.rsort[0] = -1
b.rsort
>>> array([ -1        ,  1.        ,  1.        , ..., 42.42640687, 42.42640687, 42.42640687])

By editing the (default) value of rsort in a, the value in b also changes. And that shouldn't happen.

I don't know whether it's a bug that affects Suite2p - probably not - but it was fixed in the built in library in Python 3.11, and Suite2p will have to make some change to be usable in 3.11 as a result. This change should fix it, although the extent of my testing so far is that the resulting rsort value is unchanged.

landoskape commented 8 months ago

Ah I see. I think suite2p is only supported for 3.9 (that's what it says in the README, although the instructions for developers doesn't include this). It definitely seems useful to make an update that is fully back-compatible though!

simon-ball commented 8 months ago

It definitely seems useful to make an update that is fully back-compatible though!

That was pretty much my thoughts too. I uncovered this while trying to get ahead of the impending EOL of python 3.8 (which is my current target version), and Suite2p appears to be the biggest obstruction (1) to me to choosing 3.11 as the new target. Given the work involved in re-validating everything for a new target, I'd definitely prefer to aim for 3.11 and push the next one off by another year!

(1) not intended in a negative sense - simply that it's non-optional to us right now, and it restricts us to a target no newer than 3.10.

marius10p commented 8 months ago

@simon-ball thanks for finding this problem in 3.11. Bringing our dependencies forward is a big task and we don't have the manpower to do it very often. Since 3.10 EOL is in three years, we are going to hold off on this for a bit. Please feel free to maintain your own fork of suite2p with the update.

simon-ball commented 8 months ago

Fair enough so

Bringing our dependencies forward is a big task and we don't have the manpower to do it very often

Can't disagree with that!

MouseLand / suite2p

Fix use of dataclasses for python 3.11 #1047