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

MAINT: update pytest, hypothesis, pytest-cov, and pytz in test_requirements.txt #24163

Merged
merged 2 commits into from
Jul 13, 2023

Conversation

ngoldbaum
Copy link
Member

These dependencies haven't been updated since 2021 when dependabot was disabled. Let's see if updating them causes any issues in any CI jobs.

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for updating this. Perhaps going a step further and unpinning entirely (or using lower-bounds only) could be considered? Otherwise I think there's a risk of running into a situation where we're several major versions behind pytest and it's much harder to update since we've missed all the incremental maintenance tweaks along the way. AFAIK these pins were not related to any real version incompatibilities, but please ignore the suggestion if I'm mistaken!

@ngoldbaum
Copy link
Member Author

Added a toml dependency for test_all_newsfragments_used.py, which imports toml directly. This is needed now because pytest 7 switched from toml to tomli. We could probably do the same and use tomllib on Python 3.11 or newer, but I didn't want to mess with the towncrier setup more than needed.

Happy to drop the explicit version constraints on the test dependencies I changed here if that's desired.

@Mousius
Copy link
Member

Mousius commented Jul 13, 2023

+1 for updating this. Perhaps going a step further and unpinning entirely (or using lower-bounds only) could be considered? Otherwise I think there's a risk of running into a situation where we're several major versions behind pytest and it's much harder to update since we've missed all the incremental maintenance tweaks along the way. AFAIK these pins were not related to any real version incompatibilities, but please ignore the suggestion if I'm mistaken!

Unpinning or just taking new versions can lead to instability because new versions may not actually be compatible, in semantic versioning the major version changes are breaking. Another issue is if an attacker can gain access to a dependency, they can publish a new version which gets pulled in immediately.

A better solution would be to enable dependabot or similar. Dependabot scans the requirements.txt and generates a pull request with the new version so you can easily validate the new version. That way you get rolling updates which have been validated before impacting anyone else, with a light bit of a human intervention to sense check it.

@seberg
Copy link
Member

seberg commented Jul 13, 2023

FWIW, I don't have an opinion on unpinning or dependabot or just keeping this. In practice, these dependencies probably rarely break us (and if they do, its just the testing work-flow we can quickly adapt, in the worst case a confused/frustrated contributor during a transition period -- although I am not sure that won't also happen if they are pinned :)).

We could also just set up dependabot to a very low frequency, like every 3 months or so.

@ngoldbaum do you have a preference? In that case I am happy to just go with that.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good and is green, thanks Nathan! So let's get this in; the rest of the conversation about a future plan can continue (either good as is, or trigger a follow-up PR).

@rgommers rgommers merged commit 9d8d46a into numpy:main Jul 13, 2023
57 checks passed
@rgommers rgommers added this to the 2.0.0 release milestone Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants