scikit-learn-contrib / metric-learn

Metric learning algorithms in Python
http://contrib.scikit-learn.org/metric-learn/
MIT License
1.4k stars 234 forks source link

numpy 1.26.0 does not support stacking a set of tuples #354

Closed gykovacs closed 1 year ago

gykovacs commented 1 year ago

Description

There are some minor incompatibility problems with numpy 1.26.0

Steps/Code to Reproduce

import numpy as np
import metric_learn as ml

print(np.__version__)

ml.ITML_Supervised().fit(np.random.random_sample(size=(100, 5)), np.random.randint(2, size=100))

Expected Results

The expected result is the code running.

Actual Results

The actual result is a numpy error: image

Versions

Linux-5.15.90.1-microsoft-standard-WSL2-x86_64-with-glibc2.31 Python 3.11.5 | packaged by conda-forge | (main, Aug 27 2023, 03:34:09) [GCC 12.3.0] NumPy 1.26.0 SciPy 1.11.3 Scikit-Learn 1.3.1 Metric-Learn 0.6.2


Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

perimosocordiae commented 1 year ago

Thanks for the bug report. We were using a set to enforce uniqueness of the tuples, but it might make more sense to reshape and then call np.unique(axis=0) to achieve the same effect. Should be more efficient as well.

gykovacs commented 1 year ago

Great, thank you for the response. Do you plan a release with the fix anytime soon? (No push, just want to know if I should fix the numpy version for a while or can expect it working in a couple of days)

perimosocordiae commented 1 year ago

I just tried to replicate and found that this issue was fixed by gh-339, so it seems a fresh release is long overdue!

perimosocordiae commented 1 year ago

I just created a new release: https://github.com/scikit-learn-contrib/metric-learn/releases/tag/v0.7.0

@terrytangyuan @wdevazelhes Can you help push this to PyPI and regenerate the docs? (It's been long enough that I don't remember how.)

bellet commented 1 year ago

Thanks @gykovacs for spotting this and @perimosocordiae for the new release! @terrytangyuan @wdevazelhes if one of you could also push this to the conda-forge repo (https://github.com/conda-forge/metric-learn-feedstock) in addition to PyPI that would be great :-) Thanks a lot!

wdevazelhes commented 1 year ago

Thanks @gykovacs for creating the issue and thanks @perimosocordiae for the new release and fixes 🙏 @perimosocordiae @bellet @terrytangyuan I just pushed the release on PyPI and conda-forge, and also pushed the new documentation 👍 let me know if you spot any problems 🙏 : with a quick check the doc looks ok and @gykovacs with an install with pip or conda-forge of metric-learn the bug now seems resolved 👍 🙏

gykovacs commented 1 year ago

Thank you for the quick intervention!

wdevazelhes commented 1 year ago

🙌 Closing the issue, feel free to reopen if any problem arises 👍

bellet commented 1 year ago

Thanks a lot @wdevazelhes!