Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix install #254

Merged
merged 8 commits into from
Sep 8, 2021
Merged

Fix install #254

merged 8 commits into from
Sep 8, 2021

Conversation

Catarina-Alves
Copy link
Collaborator

Fix issue preventing the installation of snmachine in NERSC: see here.

@heather999
Copy link
Contributor

I need to do some more testing in the morning, but I think we want to add a requirements.txt with the contents:

astropy>=1.1.2
matplotlib>=1.5.1
numpy>=1.18.4
scikit-learn==0.20.0
scipy>=0.17.0
george>=0.3.0
iminuit
pandas>=0.23.0
pywavelets>=0.4.0
sncosmo
nose>=1.3.7
future>=0.16
pyyaml>=3.13
seaborn
lightgbm
setuptools_scm

and then update setup.py's install_requires line:

install_requires=['astropy>=1.1.2',
                      'matplotlib>=1.5.1',
                      'numpy>=1.18.4',
                      'scikit-learn==0.20.0',
                      'scipy>=0.17.0',
                      'george>=0.3.0',
                      'iminuit',
                      'pandas>=0.23.0',
                      'pywavelets>=0.4.0',
                      'sncosmo',
                      'nose>=1.3.7',
                      'future>=0.16',
                      'pyyaml>=3.13',
                      'seaborn',
                      'lightgbm',
                      "setuptools_scm"],

I think that will limit the installed dependencies to just what is needed. In my test install, I needed to remove the constraints on the iminuit and sncosmo packages. Were those restrictions really needed?

@Catarina-Alves
Copy link
Collaborator Author

Thank you @heather999.
What was happening when the iminuit and sncosmo packages had the restrictions? We can remove the iminuit restriction but the sncosmo is important to keep, otherwise the tests start failing as sncosmo is updated because the function outputs change.

@heather999
Copy link
Contributor

It should be fine to keep the restriction on sncosmo set to 2.1.0. I had some trouble with iminuit with the restrictions due to trouble building from source and being unable to find a suitable wheel binary for install at NERSC - removing the version restriction fixed that. So here's my amended suggestion for requirements.txt:

astropy>=1.1.2
matplotlib>=1.5.1
numpy>=1.18.4
scikit-learn==0.20.0
scipy>=0.17.0
george>=0.3.0
iminuit
pandas>=0.23.0
pywavelets>=0.4.0
sncosmo==2.1.0
nose>=1.3.7
future>=0.16
pyyaml>=3.13
seaborn
lightgbm
setuptools_scm

and

install_requires=['astropy>=1.1.2',
                      'matplotlib>=1.5.1',
                      'numpy>=1.18.4',
                      'scikit-learn==0.20.0',
                      'scipy>=0.17.0',
                      'george>=0.3.0',
                      'iminuit',
                      'pandas>=0.23.0',
                      'pywavelets>=0.4.0',
                      'sncosmo==2.1.0',
                      'nose>=1.3.7',
                      'future>=0.16',
                      'pyyaml>=3.13',
                      'seaborn',
                      'lightgbm',
                      "setuptools_scm"],

Once that is set - maybe we can think about setting up automated PyPI uploads in GitHub actions - once we're sure the pip installs are generally working well.

@heather999
Copy link
Contributor

Learning some things.. I'm working with python 3.8 and joblib < 0.14 is incompatible with that version of python.. scikit-learn 0.20.0 comes with joblib 0.13.x bundled in the distribution. More recent versions of scikit-learn would fix this. Do you need scikit-learn 0.20.0? If so, then snmachine probably will not be compatible with python 3.8 until there is an update.

@Catarina-Alves
Copy link
Collaborator Author

@heather999 I do believe that I can use the most updated scikit-learn version. It might change the result of some tests but I can fix them, if that happens. The issue with a new version of sncosmo was much more delicate.
So, what is it better to use? scikit-learn>= some version?

@heather999
Copy link
Contributor

heather999 commented Sep 3, 2021

If you can remove any restriction that might be best, but at least >=0.21.0 because at that point, scikit-learn defines joblib as an external dependency and no longer distributes an older version.

@Catarina-Alves Catarina-Alves merged commit 580eb70 into main Sep 8, 2021
@Catarina-Alves
Copy link
Collaborator Author

@heather999 I have made the required changes and upload the recent version to pypi. Does it work now or do I need to change something else?

@Catarina-Alves Catarina-Alves deleted the fix-install branch September 17, 2021 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants