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

Drop Python3.8, support Pandas v2.2, reduce Warnings #1565

Merged
merged 50 commits into from
Jan 29, 2024
Merged

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Dec 18, 2023

Pull Request Checklist:

What kind of change does this PR introduce?

  • Drops Python3.8 references and marks Python3.9 as base requirement.
  • Drops older xarray and pandas support.
  • Updates documentation and CI for dropped Python3.8.
  • Raises required versions of scipy and numpy (1.9.0 and 1.20.0, respectively).
  • Removes the winter_storm indice and indicator (deprecated in xclim v0.46.0).
  • Adopts the new syntax for frequency codes:
    • "A" is no more
    • Y -> YE, Q -> QE, M -> ME
    • H -> h, T -> min, S -> s, L -> ms, U -> us, N -> ns
  • Addresses several DeprecationWarning and RuntimeWarning messages related to numpy, xarray, and pint

Does this PR introduce a breaking change?

Yes, Python3.8 is no longer supported and base dependency versions have been augmented:

  • numpy>=1.20.0
  • pandas>=2.2.0
  • scipy>=1.9.0
  • xarray>=2023.11.0

Other information:

Python3.12 support is still not possible due to missing Python3.12 builds for numba. (This is being addressed in #1613) - This has been addressed.

A Pull Request has been opened to address DeprecationWarnings stemming from numpy calls in eofs (ajdawson/eofs#148; eofs has been removed due to licensing-related issues (#1621))

@Zeitsperre Zeitsperre added standards / conventions Suggestions on ways forward API Interfacing and User Concerns labels Dec 18, 2023
@Zeitsperre Zeitsperre added this to the v0.48.0 milestone Dec 18, 2023
@Zeitsperre Zeitsperre self-assigned this Dec 18, 2023
@github-actions github-actions bot added information For development/intsructional purposes sdba Issues concerning the sdba submodule. CI Automation and Contiunous Integration docs Improvements to documenation indicators Climate indices and indicators and removed API Interfacing and User Concerns labels Dec 18, 2023
Copy link

Note
It appears that this Pull Request modifies the main.yml workflow.

On inspection, the XCLIM_TESTDATA_BRANCH environment variable is set to the most recent tag (v2023.12.14).

No further action is required.

@Zeitsperre
Copy link
Collaborator Author

@aulemahal I've left a few "FIXME" notices since changes will ultimately involve us needing to make some API/UI decisions.

environment.yml Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
@Zeitsperre Zeitsperre changed the title Drop Python3.8 Drop Python3.8 [do not merge] Dec 19, 2023
@aulemahal
Copy link
Collaborator

Fixed! My solution also works with 2023.11.0.

@Zeitsperre Zeitsperre changed the title Drop Python3.8, support Pandas v2.2 [do not merge] Drop Python3.8, support Pandas v2.2, reduce Warnings Jan 23, 2024
@Zeitsperre Zeitsperre marked this pull request as ready for review January 23, 2024 23:48
@Zeitsperre
Copy link
Collaborator Author

Adding the approved label so that we can run more tests. This PR will still require a proper review.

@Zeitsperre Zeitsperre added the approved Approved for additional tests label Jan 24, 2024
@coveralls
Copy link

coveralls commented Jan 25, 2024

Coverage Status

coverage: 90.169% (-0.1%) from 90.312%
when pulling 8bd6efc on drop-python38
into 8db48d8 on master.

Copy link
Collaborator

@aulemahal aulemahal left a comment

Choose a reason for hiding this comment

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

Some minor comments.
But I hereby approve the lines I did not write.

CONTRIBUTING.rst Outdated Show resolved Hide resolved
environment.yml Outdated Show resolved Hide resolved
CHANGES.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@huard huard left a comment

Choose a reason for hiding this comment

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

This PR mixes a lot of apparently unrelated changes (formatting, the pandas freq update, deprecations, etc). Makes it hard to review.

The code still accepts "A" as an offset, I assume it's because it's still supported upstream?

xclim/sdba/base.py Show resolved Hide resolved
Co-authored-by: Pascal Bourgault <[email protected]>
@aulemahal
Copy link
Collaborator

Sorry for the mammouth PR indeed. All those changes stem from the same two updates of python and pandas.

Xclim doesn't precribe what frequency code should be used, in most cases, it only passes them down to xarray. This PR changes all places where xclim was using the old codes (tests, checks, documentation). A user with pandas < 3 will stil be able to use "A" and should get a warning from pandas.

However, there a some instances where a frequency code is checked against the output of xr.infer_freq or parsed with pd.frequencies.to_offset. Changes there are breaking, which explains why we decided to pin pandas and xarray to the versions accepting the new codes.

@Zeitsperre Zeitsperre merged commit 4a697a9 into master Jan 29, 2024
19 checks passed
@Zeitsperre Zeitsperre deleted the drop-python38 branch January 29, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Interfacing and User Concerns approved Approved for additional tests CI Automation and Contiunous Integration docs Improvements to documenation indicators Climate indices and indicators information For development/intsructional purposes sdba Issues concerning the sdba submodule. standards / conventions Suggestions on ways forward
Projects
None yet
4 participants