Skip to content

Update unit testing to use UV over micromamba for environment management#228

Merged
ludwiglierhammer merged 11 commits intoglamod:mainfrom
jtsiddons:uvnit_testing
Feb 18, 2025
Merged

Update unit testing to use UV over micromamba for environment management#228
ludwiglierhammer merged 11 commits intoglamod:mainfrom
jtsiddons:uvnit_testing

Conversation

@jtsiddons
Copy link
Collaborator

@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.23%. Comparing base (87403d8) to head (286f27d).
Report is 22 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #228   +/-   ##
=======================================
  Coverage   84.23%   84.23%           
=======================================
  Files          76       76           
  Lines        2817     2817           
=======================================
  Hits         2373     2373           
  Misses        444      444           
Flag Coverage Δ
unittests 84.23% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Add python 3.13

Co-authored-by: Ludwig Lierhammer <Ludwig.Lierhammer@dwd.de>
@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@ludwiglierhammer
Copy link
Collaborator

Once you have the unit tests running, I would implement a new Github action that tests if the environment creation works with both pip and conda for different Python versions.

@ludwiglierhammer
Copy link
Collaborator

py3.9 has failed due to connection problems:
http.client.RemoteDisconnected: Remote end closed connection without response

In general, the tests are passing 🎉

@jtsiddons
Copy link
Collaborator Author

@ludwiglierhammer

Once you have the unit tests running, I would implement a new Github action that tests if the environment creation works with both pip and conda for different Python versions.

Sure, are you thinking a check similar to

pip install .
python -c "import cdm_reader_mapper"

And similar for conda?

@ludwiglierhammer
Copy link
Collaborator

@ludwiglierhammer

Once you have the unit tests running, I would implement a new Github action that tests if the environment creation works with both pip and conda for different Python versions.

Sure, are you thinking a check similar to

pip install .
python -c "import cdm_reader_mapper"

And similar for conda?

Yes, I would create a new yaml file in .github/workflows.

@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@jtsiddons
Copy link
Collaborator Author

@ludwiglierhammer

Once you have the unit tests running, I would implement a new Github action that tests if the environment creation works with both pip and conda for different Python versions.

Sure, are you thinking a check similar to

pip install .
python -c "import cdm_reader_mapper"

And similar for conda?

Yes, I would create a new yaml file in .github/workflows.

I've added a workflow for installation check via pip for py3.11.

It creates a conda envrionment with no dependencies (other than the python version). It then installs the library with pip install .

See https://github.com/glamod/cdm_reader_mapper/actions/runs/13367474700/job/37328361995?pr=228

Thoughts/suggestions?

@ludwiglierhammer
Copy link
Collaborator

I did a new PR (#229) regarding exactly this issue. Looks similar to your install.yml.

@ludwiglierhammer
Copy link
Collaborator

I did a new PR (#229) regarding exactly this issue. Looks similar to your install.yml.

Unfortunately, it is not running at the moment.

@ludwiglierhammer
Copy link
Collaborator

@jtsiddons: I updated some dependencies in PR #229 and did some renamings:

  • .github/workflows/ci.yml -> .github/workflows/testing-suite.yml
  • ci/requirements -> CI

Since cdm_reader_mapper is not deployed to conda, we do not need a environment creation checker, I think.

@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@jtsiddons
Copy link
Collaborator Author

@ludwiglierhammer: Did the tests change or did I merge main incorrectly?

@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@ludwiglierhammer
Copy link
Collaborator

@ludwiglierhammer: Did the tests change or did I merge main incorrectly?

Sorry, my bad. I fixed this.

@jtsiddons
Copy link
Collaborator Author

jtsiddons commented Feb 18, 2025

Do we want to add any additional workflows?

I can't think of any. I have no further workflows in my mind.
I usuallly get ideas for workflows from xclim.

Do you have any specific ideas?

Co-authored-by: Ludwig Lierhammer <Ludwig.Lierhammer@dwd.de>
@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@jtsiddons
Copy link
Collaborator Author

jtsiddons commented Feb 18, 2025

Do we want to add any additional workflows?

I can't think of any. I have no further workflows in my mind. I usuallly get ideas for workflows from xclim.

Do you have any specific ideas?

Not immediately. I can mark this as ready for review if you're happy.

Looks good to me 👍

Co-authored-by: Ludwig Lierhammer <Ludwig.Lierhammer@dwd.de>
@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@jtsiddons jtsiddons marked this pull request as ready for review February 18, 2025 09:06
@github-actions
Copy link

Warning
This Pull Request is coming from a fork and must be manually tagged approved
in order to perform additional testing.

@jtsiddons
Copy link
Collaborator Author

jtsiddons commented Feb 18, 2025

Do you want me to update changes.rst?

Yes, gladly. I thought of somethign like this:

Announcements
^^^^^^^^^^^^^
This release adds support for Python 3.13 (:pull:228)

@jtsiddons
Copy link
Collaborator Author

jtsiddons commented Feb 18, 2025

Does this want to be under a new (unreleased) section? Should I add the change in CI
environment mangement to the internal changes sub-section?

I would call the new release 2.0.1 since it it just a minor revision for the users.
I would be happy if you could add the CI changes. I forgot yesterday to add them yesterday.

E.g:

2.0.1 (unreleased)
------------------
Contributors to this version: Ludwig Lierhammer (:user:`ludwiglierhammer`) and Joseph Siddons (:user:`jtsiddons`)

Announcements
^^^^^^^^^^^^^
This release drops support for Python 3.9 and adds support for Python 3.13 (:pull:`228`, :pull:`229`)

New features and enhancements
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
* add environment.yml file (:pull:`229`)

Internal changes
^^^^^^^^^^^^^^^^
* GitHub workflow for ``testing_suite`` now uses ``uv`` for environment management, replacing ``micromamba`` (:pull:`228`)
* rename ci/requirements to CI and tidy up requirements/dependencies (:pull:`229`)

edits: add example and pull ref

@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

Co-authored-by: Ludwig Lierhammer <Ludwig.Lierhammer@dwd.de>
@github-actions
Copy link

Warning

This Pull Request modifies GitHub Workflows and is coming from a fork.
It is very important for the reviewer to ensure that the workflow changes are appropriate.

@ludwiglierhammer
Copy link
Collaborator

ludwiglierhammer commented Feb 18, 2025

Looks good to me. Thanks for your work. Anything to add from your side? Can I merge this PR?

@jtsiddons
Copy link
Collaborator Author

Looks ood to me. Thanks for your work. Anything to add from your side? Can I merge this PR?

I think so. I can't think of anything to add immediately.

@ludwiglierhammer ludwiglierhammer merged commit e444bbe into glamod:main Feb 18, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants