BiAPoL / napari-clusters-plotter

A napari plugin for clustering objects according to their properties.
BSD 3-Clause "New" or "Revised" License
80 stars 10 forks source link

Remove pinned numpy version when the problem is fixed (UMAP not working with Numpy>1.23.5) #215

Open jo-mueller opened 1 year ago

jo-mueller commented 1 year ago

Hi,

I was just testing some things and noticed that the UMAP was not functional in a fresh devbio-napari installation.

To reproduce

I create some sample data with this code:

import napari
from skimage import data, measure, filters
from napari_clusters_plotter._dimensionality_reduction import DimensionalityReductionWidget

image = data.human_mitosis()

labels = measure.label(image > filters.threshold_otsu(image))
features = measure.regionprops_table(labels, image, properties=('label', 'area', 'eccentricity', 'mean_intensity'))

viewer = napari.Viewer()
viewer.add_image(image, name='human mitosis')
viewer.add_labels(labels, name='human mitosis labels', features=features)

widget = DimensionalityReductionWidget(viewer)
viewer.window.add_dock_widget(widget)

napari.run()

When you select the UMAP algoithm in the dropwdown, al the features in the features list and click on run, nothing happends, whereas tSNE and PCA work. When using the debugger to step through the code, I found that this line throws an error:

result = umap(
    properties_to_reduce,
    n_neigh=n_neighbours,
    n_components=n_components,
    verbose=False,
                )

The error comes from numba: "SystemError: initialization of _internal failed without raising an exception". The error has been described here, with a possible fix being to downgrade numpy to 1.23.5.

Doing mamba install numpy==1.23.5 in my environment fixed the issue for me.

Possible fix

Pinning numpy to `numpy<1.24.0´ could do the trick, although I don't like pinning numpy. he problem has been observed in quite a few packages, though.

ziw-liu commented 2 months ago

Can this pin be removed now? NumPy 1.23 is not supported per SPEC-0 and is incompatible with other packages (e.g. latest zarr).

Cryaaa commented 1 month ago

hey @ziw-liu, thanks for catching this, I'm pretty sure that it should not cause a problem but let me test it first! It will definitely be removed in the overhaul that we are working on atm but I thinks it's a good idea to update it in the current version as well.