-
Notifications
You must be signed in to change notification settings - Fork 0
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
Support Python 3.6+ #13
Conversation
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.
Thanks!
I have one suggestion to improve reproducibility of the CI builds, but that doesn't block Python 3.6 support.
tests/requirements.txt
Outdated
@@ -1 +1,2 @@ | |||
gemmi | |||
pytest |
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.
To make CI tests more reproducible, you should pin specific versions of packages in requirements.in
, then use pip-compile
to generate requirements.txt
that pins all transitive dependencies. That way, an update to a transitive dependency is less likely to cause CI failures from one build to the next with no changes to parsnip. Readthedocs has a nice write up: https://docs.readthedocs.io/en/stable/guides/reproducible-builds.html#pin-your-transitive-dependencies
Dependabot knows how to update requirements.in
and rerun pip-compile
. Add pip to the dependabot config and you will get PRs that test the updated pins: https://github.com/glotzerlab/gsd/blob/trunk-patch/.github/dependabot.yml
Legacy support is a nice-to-have for a lightweight package like this.
Test configuration
To avoid wasted CI time, I have split the test runs into two groups. Modern python versions (3.9+) are run first on Ubuntu-latest. If these pass, a second batch is run on legacy versions 3.6-3.8. The code to run tests is consolidated into a workflow callable which takes in the python version and runner os and generates a job for each combination of input matrixes. Once the package is open source, we can test on Windows (modern and legacy) plus macOS arm (modern).
Can we go lower?
Maybe, but not easily. The tests are based off of Gemmi, which is not available below python 3.6. In addition, f-strings were introduced in python 3.6.The CI does not build, as the pypi link was unable to be found. This could be made to work, but we're getting into the realm of changes that make the experience worse on modern versions for very little benefit. If anyone has a real use case for this I will revisit the question.
Motivation and Context
Resolves #10
Types of Changes
Checklist: