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

ci: rename workflow names, revise uv setup, fix python versions #2435

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

mwtoews
Copy link
Contributor

@mwtoews mwtoews commented Feb 5, 2025

This PR does the following:

  • Rename the workflow names to be shorter, by removing "FloPy " from each
  • Remove actions/setup-python where astral-sh/setup-uv is used (mind you, both can be used according to docs)
  • Fix Python versions in matrix jobs (these were previously all running Python 3.13, according to logs)
  • Use uvx to run uv tools like ruff and twine; this may eventually phase-out the build and lint extras in pyproject.toml
  • Use uv publish instead of pypa/gh-action-pypi-publish
  • Add etc/github-develop-requirements.txt for the GitHub-linked source installs, using preferred "git+https" protocols
  • With the astral-sh/setup-uv action, specify either cache-dependency-glob: "**/pyproject.toml" or enable-cache: false to disable warning messages

Copy link

codecov bot commented Feb 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.3%. Comparing base (bb9824e) to head (9b10eb0).
Report is 47 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #2435     +/-   ##
=========================================
+ Coverage     68.4%   76.3%   +7.8%     
=========================================
  Files          294     294             
  Lines        59390   59936    +546     
=========================================
+ Hits         40652   45741   +5089     
+ Misses       18738   14195   -4543     

see 248 files with indirect coverage changes

@mwtoews mwtoews requested a review from wpbonelli February 5, 2025 01:36
uses: astral-sh/setup-uv@v5
with:
version: "0.5.18"
Copy link
Member

Choose a reason for hiding this comment

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

they also say to keep the version locked, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see this mentioned. It defaults to the latest version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, you're probably thinking of this usage doc for setup-pixi. I'd be inclined to lock this for setup-uv if it proves to be instable. On the plus side, I see very active version turn-over fixing bugs with both uv and setup-uv that I wouldn't anticipate too much instability (or short-lived instability, if any).

Copy link
Member

@wpbonelli wpbonelli left a comment

Choose a reason for hiding this comment

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

Thanks @mwtoews I learned some things here. By the way, do you agree with #2433?

@mwtoews
Copy link
Contributor Author

mwtoews commented Feb 5, 2025

I've been on a uv learning quest for the past few weeks too!

As for the uv.lock file, the lockfile docs state:

This file should be checked into version control, allowing for consistent and reproducible installations across machines.

However, generally I share the same view that lockfiles are more for applications than libraries, thus I"m somewhat neutral/undecided at this stage. Was this lockfile creating any issues or confusion in this repo?

@wpbonelli
Copy link
Member

wpbonelli commented Feb 5, 2025

Was this lockfile creating any issues or confusion in this repo?

No, it's just big, and it introduces the question of when to update it. All at once, every once in a while, I guess is the pattern for "applications". But I figured better to get an early warning when a dependency breaks for a library?

@wpbonelli wpbonelli merged commit a2f6002 into modflowpy:develop Feb 5, 2025
23 checks passed
@mwtoews mwtoews deleted the ci-revise-uv branch February 5, 2025 17:27
@mwtoews
Copy link
Contributor Author

mwtoews commented Feb 6, 2025

No, it's just big, and it introduces the question of when to update it.

Aggrege it's a big and unpleasant-looking file to add to version control. I don't know the protocol of updating it either, but I'll try to look out for this etiquette. There is a convenient-looking uv-pre-commit that keeps this updated at all times.

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.

2 participants