-
Notifications
You must be signed in to change notification settings - Fork 341
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
Move requirements-min.txt, split deps in multiple files #605
Conversation
@@ -36,7 +36,8 @@ jobs: | |||
python-version: '3.10' | |||
- name: Install pip dependencies | |||
run: | | |||
pip install -r requirements/requirements.txt | |||
pip install --pre 'rasterio>=1.0.16' | |||
pip install .[datasets,tests] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably better to go back to installing from setup.cfg
during release testing. We want to make sure this works since it's how actual PyPI installs will work.
@@ -96,6 +96,8 @@ datasets = | |||
# https://github.com/brianhelba/zipfile-deflate64/issues/19 | |||
zipfile-deflate64>=0.2,<0.3 | |||
docs = | |||
# ipywidgets 7+ required by nbsphinx | |||
ipywidgets>=7,<8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this in #574 (comment) because I couldn't remember why it was required and it didn't seem to be needed anymore. Turns out the addition of black[jupyter]
installed ipywidgets behind the scenes, so that's why the tests didn't fail. Splitting things up into separate files uncovered spatialaudio/nbsphinx#24 (comment) which explains why I added this in the first place.
open3d==0.14.1;python_version<'3.10' | ||
opencv-python==4.6.0.66 | ||
pandas==1.4.2;python_version>='3.8' | ||
pandas==1.3.5;python_version=='3.7' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's still unclear if dependabot will actually update lines like this, or if it will try to update both. Ideally, it would only update the first occurrence, or somehow parse the environment marker. If that doesn't work, we can create separate files for Python 3.7 (and 3.8 once numpy and friends drop that too).
Awesome, thanks for digging so far into this @adamjstewart! |
Turns out dependabot actually does support multiple files! I noticed this in #603 when it accidentally updated
requirements-min.txt
too. This PR does two things:requirements-min.txt
to a subdirectory to avoid dependabot updatesrequirements.txt
into separate files for each CI stepIf
requirements/min/all.txt
doesn't avoid dependabot, I'll try moving it to a different directory.Splitting
requirements.txt
into separate files fulfills @weiji14's vision from #553! Style tests now take 15 s instead of 2 min.