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

Support Python 3.6+ #13

Merged
merged 27 commits into from
May 16, 2024
Merged

Support Python 3.6+ #13

merged 27 commits into from
May 16, 2024

Conversation

janbridley
Copy link
Collaborator

@janbridley janbridley commented May 10, 2024

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

  • Documentation update
  • Bug fix
  • New feature

Checklist:

  • I am familiar with the Development Guidelines
  • The changes introduced by this pull request are covered by existing or newly introduced tests.
  • I have updated the changelog and added my name to the credits.

@janbridley janbridley marked this pull request as ready for review May 10, 2024 20:51
@janbridley janbridley marked this pull request as draft May 10, 2024 20:55
@janbridley janbridley marked this pull request as ready for review May 10, 2024 23:36
@janbridley janbridley self-assigned this May 10, 2024
@janbridley janbridley changed the title Support python 3.6 and later Support Python 3.6+ May 11, 2024
@janbridley janbridley requested a review from joaander May 15, 2024 20:04
Copy link
Member

@joaander joaander left a 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.

@@ -1 +1,2 @@
gemmi
pytest
Copy link
Member

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

@janbridley janbridley merged commit ee5894e into main May 16, 2024
7 checks passed
@janbridley janbridley deleted the feature/#10-support-python-3.6 branch May 16, 2024 17:24
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.

Support Python 3.6 and 3.7
2 participants