IAMconsortium / pyam

Analysis & visualization of energy & climate scenarios
https://pyam-iamc.readthedocs.io/
Apache License 2.0
226 stars 118 forks source link

Un-pin dependency `pint <= 0.17` #639

Closed LauWien closed 2 years ago

LauWien commented 2 years ago

Description of PR

This PR un-pins the dependency pint <= 0.17, which got introduced in #589. Closes #638.

codecov[bot] commented 2 years ago

Codecov Report

Merging #639 (345ee8a) into main (56b97d0) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #639   +/-   ##
=====================================
  Coverage   94.5%   94.5%           
=====================================
  Files         57      57           
  Lines       5663    5663           
=====================================
  Hits        5357    5357           
  Misses       306     306           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 56b97d0...345ee8a. Read the comment docs.

danielhuppmann commented 2 years ago

So the failing test reminded me why I didn't remove the pin at the time. Can you please insert minimum requirements for pint and iam-units and update the legacy-dependency test?

Please also add a section "## Dependency changes" to the release notes?

danielhuppmann commented 2 years ago

The legacy-dependency tests specifically takes the oldest supported version of several important packages, to make sure that our information on "oldest supported version" is actually correct.

If you look at the log of the GitHub action workflow, it first installed pint 0.17, and then installed pyam - but because pint 0.17 is not in line with the stated pyam requirements, it actually installed the latest version of pint (currently 0.18).

LauWien commented 2 years ago

More importantly: there was no functional change, so I suspect that the current codebase is actually compatible with pint 0.17 (or even earlier). Did you check? The iam-units seems to be backward-compatible back to pint 0.11.

Ah I see, what I checked before is, that with https://github.com/IAMconsortium/units/releases/tag/v2021.11.12 iam-units was made compatible with pint version 0.18, but checking the setup.cfg this should be right as you say, compatible back to pint 0.11. So I would then again remove the minimum dependency on pint as well as then pint==0.18 in the legacy-dependency test, leave the minimum dependency on iam-units >= 2021.11.12 and change the legacy-dependency test to iam-units==2021.11.12. What do you think?

LauWien commented 2 years ago

Superseded by https://github.com/IAMconsortium/pyam/pull/640.