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

Pylint fixes, remove deprecated functions #1881

Merged
merged 31 commits into from
Sep 4, 2024
Merged

Pylint fixes, remove deprecated functions #1881

merged 31 commits into from
Sep 4, 2024

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Aug 13, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGELOG.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Adjust code base to address several pylint-related warnings.
  • Removes several deprecated calendar functions.
  • Pins the vulture dependency in environment.yml
  • Employed casting to deal with a few numpy * xarray typing issues.
  • Addressed a couple warnings related to pint usage.

Does this PR introduce a breaking change?

Yes, several calendar-related functions that were deprecated and slated for removal in xclim v0.50 and v0.51 have been removed. These will need to be listed in the CHANGELOG.rst.

Other Information:

I'm now a conda-forge maintainer for vulture, haha (https://github.com/conda-forge/vulture-feedstock)

@Zeitsperre Zeitsperre self-assigned this Aug 13, 2024
@github-actions github-actions bot added sdba Issues concerning the sdba submodule. API Interfacing and User Concerns docs Improvements to documenation indicators Climate indices and indicators labels Aug 13, 2024
@Zeitsperre Zeitsperre requested review from aulemahal and coxipi August 15, 2024 16:29
@Zeitsperre Zeitsperre changed the title Pylint fixes Pylint fixes, remove deprecated functions Aug 15, 2024
xclim/indices/_agro.py Outdated Show resolved Hide resolved
@Zeitsperre Zeitsperre marked this pull request as ready for review August 16, 2024 14:37
docs/conf.py Show resolved Hide resolved
xclim/sdba/measures.py Outdated Show resolved Hide resolved
Copy link
Contributor

@coxipi coxipi left a comment

Choose a reason for hiding this comment

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

looks good to me, but maybe double check the loops that you changed

@Zeitsperre Zeitsperre requested a review from coxipi August 22, 2024 14:18
@Zeitsperre
Copy link
Collaborator Author

@aulemahal @coxipi This is good for final review!

xclim/core/units.py Outdated Show resolved Hide resolved
xclim/core/units.py Outdated Show resolved Hide resolved
xclim/core/utils.py Show resolved Hide resolved
xclim/indices/_conversion.py Show resolved Hide resolved
xclim/sdba/utils.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the approved Approved for additional tests label Aug 22, 2024
Co-authored-by: Pascal Bourgault <[email protected]>
@coxipi coxipi mentioned this pull request Aug 22, 2024
5 tasks
Copy link
Contributor

@coxipi coxipi left a comment

Choose a reason for hiding this comment

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

approved

@Zeitsperre
Copy link
Collaborator Author

The notebooks have been fixed in #1889. It likely makes more sense to merge that PR first before this one.

@coveralls
Copy link

coveralls commented Aug 29, 2024

Coverage Status

coverage: 89.393% (+0.2%) from 89.155%
when pulling faec7d4 on pylint-fixes
into de8b8b8 on main.

…ons to _types.py and _exceptions.py, use absolute imports
…ed operations in helpers.py, update __all__ in stats.py, other small fixes
@Zeitsperre
Copy link
Collaborator Author

@aulemahal

I've made one or two small breaking changes:

  • I've migrated xclim.core to use absolute imports instead of explicit relative imports (from xclim.core import ...)
  • I've factored out a few small objects into hidden _types.py and _exceptions.py under xclim.core. The goal here being to reduce the number of time xclim.core.utils is imported throughout the module via its many members. Technically, this is breaking (and there are possibly other objects that might make sense being moved).

The rest of the changes are to clarify internal logic within functions via renaming. Thanks in advance for the re-review

@Zeitsperre Zeitsperre requested a review from aulemahal August 29, 2024 17:39
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.

Ah! Well done, thanks.

@Zeitsperre Zeitsperre merged commit 15b3d07 into main Sep 4, 2024
21 checks passed
@Zeitsperre Zeitsperre deleted the pylint-fixes branch September 4, 2024 13:58
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 docs Improvements to documenation indicators Climate indices and indicators sdba Issues concerning the sdba submodule.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants