search

PetroFit / petrofit

Python package for calculating Petrosian properties and fitting galaxy light profiles
https://petrofit.readthedocs.io
26 stars 6 forks source link

sklearn is deprecated -- use scikit-learn instead #186

Closed SterlingYM closed 1 year ago

SterlingYM commented 1 year ago

PyPi package sklearn is deprecated (see https://pypi.org/project/sklearn/). This raises an exception when importing the package (e.g., from sklearn.tree import DecisionTreeRegressor in petrosian.py:9). Please consider switching to scikit-learn instead.

robelgeda commented 1 year ago

Thank you! I am planning on introducing major upgrades for v0.5.0 soon (hoping within the next two weeks) and will make sure this is fixed!

robelgeda commented 1 year ago

I just realized that it has been fixed by #174!

SterlingYM commented 1 year ago

Sorry for the duplicate, I didn't realize there was an existing pull request. Looking at #174, however, I only see the setup.cfg being changed, and this does not actually solve the problem as Petrofit still tries to import sklearn. I did not have this issue when I installed Petrofit on my existing environment where I had scikit-learn installed, but I got the error message when I did the following to create the new environment.

Minimum example:

conda create -n petrofit python=3.10
conda activate petrofit
pip install petrofit
pip install matplotlib # by the way matplotlib is missing in the requirement
python -c "import petrofit"

Output: ModuleNotFoundError: No module named 'sklearn'

I believe there's still an issue with sklearn since pip says Successfully installed PyWavelets-1.4.1 astropy-5.3.2 imageio-2.31.1 lazy_loader-0.3 networkx-3.1 numpy-1.25.2 packaging-23.1 petrofit-0.4.1 photutils-1.9.0 pillow-10.0.0 pyerfa-2.0.0.3 pyyaml-6.0.1 scikit-image-0.21.0 scipy-1.11.2 sklearn-0.0.post7 tifffile-2023.8.12.

Notice it's installing sklearn-0.0.post7, and manually downgrading sklearn to pip install -U sklearn==0.0 solves the issue.

robelgeda commented 1 year ago

Thank you for pointing that, I am going to reopen this ticket because we may need to update and release a minor version for 0.4.x.

robelgeda commented 1 year ago

Thank you so much, I just realized this was a bug and that the import statement was not removed during the last refactor. This was actually a bug in the main branch not v0.4.1. #192 fixes the issue.