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

Make xESMF optional #337

Merged
merged 9 commits into from Feb 7, 2024
Merged

Make xESMF optional #337

merged 9 commits into from Feb 7, 2024

Conversation

aulemahal
Copy link
Collaborator

@aulemahal aulemahal commented Feb 6, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been updated.
  • This PR does not seem to break the templates.
  • CHANGES.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?

  • Make xesmf optional:
    • Remove it from the dependencies in "pyproject.toml". It is now an "extra" dependency. I did not remove it from the environment.yml files as the issue doesn't occur with conda.
    • Add a few "try: import" throughout the package and raise on call time when xesmf is not available.
    • Skip the tests that require xesmf, if not present.
    • Edit the tox config to remove any usage of conda, since it is now not needed.

Does this PR introduce a breaking change?

No.

Other information:

@Zeitsperre I may have been heavy handed in editing the workflow and tox configuration. My idea was that the "test-pypi" didn't need xesmf/esmf anymore and so I removed any mention of it and of mamba/conda. So it can really be a "test-pypi" ci.

@aulemahal
Copy link
Collaborator Author

Yes ça marche!

Issue with Python 3.9 is because of the latest conda package of intake-esm. They dropped 3.9 but the conda recipe didn't know about that.

Copy link
Contributor

@RondeauG RondeauG left a comment

Choose a reason for hiding this comment

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

Thanks! I think this will make things much easier.

@aulemahal
Copy link
Collaborator Author

CI fails because the misconfigured conda package still exists in mamba's cache and in the conda-forge repo. But it did pass when I pinned intake-esm.

Coverage for Pypi runs are decreased, which is normal since we don't test any function using xesmf. But the conda converage is stable. @Zeitsperre , what should we do ? Accept the decrease ? Stop reporting coverage for those tests ?

But otherwise, this PR is good to go.

tox.ini Outdated Show resolved Hide resolved
@Zeitsperre
Copy link
Contributor

@Zeitsperre , what should we do ? Accept the decrease ? Stop reporting coverage for those tests ?

I think accepting the one-time decrease is fine. So long as the overall coverage doesn't go down (test-pypi + test-conda), that's what matters.

@aulemahal aulemahal merged commit a40422a into main Feb 7, 2024
17 of 20 checks passed
@aulemahal aulemahal deleted the opt-xesmf-pip branch February 7, 2024 17:43
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.

Utilisation / installer de xhydro (ou une partie) avec Pip
3 participants