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

Reduce dependencies and relax pins for easier installation #23

Closed
scottyhq opened this issue Apr 6, 2021 · 9 comments
Closed

Reduce dependencies and relax pins for easier installation #23

scottyhq opened this issue Apr 6, 2021 · 9 comments

Comments

@scottyhq
Copy link

scottyhq commented Apr 6, 2021

https://github.com/hpmarshall/SnowEx2020_SQLcode/blob/master/requirements.txt has many optional dependencies that could be removed or use more relaxed pins to make installation easier in existing python environments.

One issue that arises from the current setup is that installing running on jupyterhub running jlab>=3 can install an incompatible version of jupyterlab:
https://github.com/hpmarshall/SnowEx2020_SQLcode/blob/2fb72ec2dc5f48fd8bd2b207553f90a05100ff75/requirements.txt#L10
One option for all the visualization-related libraries is to define them as optional dependencies:
https://stackoverflow.com/questions/6237946/optional-dependencies-in-distutils-pip

For pinning specific versions for a consistent development environment, consider using https://python-poetry.org or https://pip.pypa.io/en/stable/reference/pip_freeze/ or https://github.com/conda-incubator/conda-lock to generate lock files

@scottyhq
Copy link
Author

scottyhq commented Apr 6, 2021

A short term solution is to change requirements.txt to just include the bare minimum without pins or with minimal pins:

geoalchemy2
geopandas>=0.7
snowmicropyn
psycopg2-binary

cc @lsetiawan @micahjohnson150

@scottyhq
Copy link
Author

scottyhq commented Apr 15, 2021

@hpmarshall @micahjohnson150 could also consider renaming the repository to match python packaging conventions

so 'SnowEx2020_SQLcode'-->'snowXSQL' so that installation instructions are pip install git+https://github.com/hpmarshall/snowxsql@master

and then from a notebook or script it's import snowxsql as in your documented examples

@hpmarshall
Copy link
Member

hpmarshall commented Apr 16, 2021 via email

@scottyhq
Copy link
Author

Anything you can think of that might break by
renaming a repository that we should be aware of?

Nope, shouldn't be an issue since there aren't any forks yet see docs here https://docs.github.com/en/github/administering-a-repository/renaming-a-repository

you just have to update some of your links:
https://github.com/hpmarshall/SnowEx2020_SQLcode/search?q=SnowEx2020_SQLcode

@micahjohnson150
Copy link
Member

@scottyhq update here, the repo name has been changed and I am working on relaxing the dependencies. I had a couple tests start failing so I need to fix those before I finish.

@micahjohnson150
Copy link
Member

@scottyhq I believe we have satisfied everything this morning. Repo name was changed and all references. Dependencies were relaxed.

@scottyhq
Copy link
Author

scottyhq commented May 2, 2021

thanks @micahjohnson150 ! I think there are still some simplifications that can be made to further reduce dependencies in the requirements.txt.

I'd suggested moving anything that is related to user-interface and development/testing to requirements-dev.txt. It'd be great to limit requirements.txt to whatever the snowexsql package itself imports, rather than including additional packages typically used for working with snowexsql.

This is helpful for example if i'd like to install and work with snowexsql in an ipython environment (no jupyterlab). You can also use minimum version pins to facilitate adding snowexsql to other python environments (so pandas>=1 instead of pandas==1.2.4). related to: https://github.com/snowex-hackweek/docker-image/pull/5/files

So i'd propose changing the following:

coloredlogs==14.0
pandas==1.2.4
coloredlogs==14.0
progressbar2==3.51.3
matplotlib==3.2.2
rasterio==1.1.5
utm==0.5.0
jupyterlab==2.1.5
descartes==1.1.0
pandoc==1.0.2
nbsphinx==0.7.1
sphinx-gallery==0.7.0
nbconvert==6.0.6
geoalchemy2
geopandas>=0.7
snowmicropyn
psycopg2-binary

to:

geoalchemy2>=0.6
geopandas>=0.7
numpy>=1
matplotlib>=3
utm>=0.5
pytz

@micahjohnson150
Copy link
Member

@scottyhq you are right there is some stuff in here that definitely belongs in requirements_dev.txt, I must of done a pip freeze a while ago.

I am a little hesitant to leave pins open upward like this especially for projects < 1.0. I have been burned in the past by updates that broke things. I am happy to try and match the hub requirements more closely though.

Another issue I am running against is the repo is two parts. 1. for building the db and uploading data to it. 2. for accessing it. Unfortunately they're both intertwined here so things like coloredlogs and progress bar are going to stay, which I don't think is too problematic (?).

What do you think about the following?

coloredlogs>=14.0
progressbar2>=3.51.3
rasterio>=1.1.5
utm>=0.5.0,<1.0
geoalchemy2 >=0.6, <1.0
geopandas>=0.7,<1.0
psycopg2>=2.8.6

@scottyhq
Copy link
Author

scottyhq commented May 3, 2021

What do you think about the following?

sounds good.

  1. for building the db and uploading data to it. 2. for accessing it. Unfortunately they're both intertwined here so things like coloredlogs and progress bar are going to stay, which I don't think is too problematic (?).

definitely something to keep in mind for your development roadmap if you want to separate the two, but for the upcoming hackweek the change you mentioned above would be fine.

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

No branches or pull requests

3 participants