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

Update requirements in pyproject.toml #275

Merged
merged 6 commits into from
Oct 18, 2023
Merged

Conversation

znicholls
Copy link
Collaborator

@znicholls znicholls commented Oct 18, 2023

Pull request

Loosen requirements of everything.

This would be a clarification of our version support policy, essentially that scmdata is intended as a library so has loose version requirements to make life easier for downstream users, with the tradeoff being that we don't guarantee installations will just work, for example we don't pin the maximum version of dependencies so in the case where a package comes out with a new major version things might break and users would have to do that pin themselves until we push a fix (which we should capture somewhere). We probably also need to somehow test that our minimum supported versions are actually supported (and perhaps adding scheduled CI to check more quickly if a new release of a package broke things). Checking minimum supported versions is a bit tricky: do you support all the minimums being installed simultaneously or them being installed one by one (with the default version of everything else)?

This is required to have a chance at passing iiasa/climate-assessment#46

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Example added (either to an existing notebook or as a new notebook, where applicable)
  • Changelog in '/changelog' added

@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (44886a0) 95.15% compared to head (bf80b14) 95.15%.
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #275   +/-   ##
=======================================
  Coverage   95.15%   95.15%           
=======================================
  Files          24       24           
  Lines        2190     2190           
  Branches      404      404           
=======================================
  Hits         2084     2084           
  Misses         86       86           
  Partials       20       20           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@mikapfl mikapfl left a comment

Choose a reason for hiding this comment

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

lgtm

@znicholls
Copy link
Collaborator Author

@mikapfl I updated the PR description, would be interested in your thoughts.

@mikapfl
Copy link
Member

mikapfl commented Oct 18, 2023

Hi,

I think it makes sense to loosen requirements here, but also put in version pins when we know something is broken. So, if a new pandas is released which breaks scmdata, we can put in a version pin first, then work on a fix. Given the fast movement and rather good compatibility of a lot of the python ecosystem, I think this reactive approach is more viable for a library than pinning versions proactively.

OTOH, to keep the test matrix somewhat sane, I'd follow NEP29 and drop Python and NumPy version support aggressively along NEP29's lines, i.e. introduce minimum version numbers for Python and NumPy even without known problems.

Regarding better QA: Definitely would be worthwhile to run some QA on a weekly or so schedule, we discussed this previously for primap: primap-community/primap2#130
Our upstreams would love us to run development versions of their libraries, which ideally leads to less breakage overall.

Cheers

Mika

@lewisjared
Copy link
Collaborator

I think we should follow NEP29 rather than trying to support older versions. I would rather put energy into adding features instead of trying to work around older versions. We shouldn't make our lives harder than they need to be

If a python 3.8 user needs to use ScmData there are a number of old releases that satisfy their needs.

And a +1 for weekly CI

@znicholls
Copy link
Collaborator Author

znicholls commented Oct 18, 2023

Ok great thanks both. I'll do the following then:

@znicholls znicholls marked this pull request as ready for review October 18, 2023 10:38
@znicholls znicholls merged commit 46f7bf2 into master Oct 18, 2023
16 checks passed
@znicholls znicholls deleted the loosen-requirements branch October 18, 2023 13:17
@phackstock
Copy link

Thanks a lot everyone for the quick fix.

cftime = ">=1.5"
numpy = "*"
openscm-units = "*"
packaging = "*"
pandas = ">=1.1"
pint = "<0.20"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought that the latest version of pint was incompatible with scmdata without changing a bunch of tests. Was that not true @znicholls ?

Copy link
Collaborator Author

@znicholls znicholls Oct 25, 2023

Choose a reason for hiding this comment

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

I think there was a double fix required i.e. once pint AND pint-pandas (or pandas) got updated, then the whole ecosystem started behaving again

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.

4 participants