Skip to content

ci: Switch from hatch, pip to uv in GitHub Actions#3719

Merged
dangotbanned merged 16 commits intovega:mainfrom
dangotbanned:uv-github-actions
Dec 20, 2024
Merged

ci: Switch from hatch, pip to uv in GitHub Actions#3719
dangotbanned merged 16 commits intovega:mainfrom
dangotbanned:uv-github-actions

Conversation

@dangotbanned
Copy link
Copy Markdown
Member

@dangotbanned dangotbanned commented Dec 20, 2024

Closes #3718

One step towards #3577

This PR moves to uv in each of:

I tried this a while back, but gave up on (27999c1).
Since then, I've used uv some more as I mentioned in (#3577 (comment)) - which helped unblock this part 😅

Replaced the `hatch` scripts with the actual commands
@dangotbanned dangotbanned enabled auto-merge (squash) December 20, 2024 22:00
@dangotbanned dangotbanned merged commit 1208c5d into vega:main Dec 20, 2024
@dangotbanned dangotbanned deleted the uv-github-actions branch December 20, 2024 22:24
on: [push, pull_request]

env:
UV_SYSTEM_PYTHON: 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great PR @dangotbanned! Do you have a reference why this is environment definition is necessary? Thanks!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks @mattijn!
Yeah I can explain.

I've enabled UV_SYSTEM_PYTHON as a workflow environment variable to add the --system argument on all relevant uv commands.
You can see in narwhals/.github/workflows/downstream_tests.yml that argument is used 25 times.

So this is just to keep it simple, making sure it is there when needed - but isn't something you need to worry about if updating the workflow in the future 🙂

@mattijn
Copy link
Copy Markdown
Contributor

mattijn commented Dec 21, 2024

Regarding feedback on parallel running of sphinx docs:

WARNING: the sphinxext_altair.altairplot 
extension does not declare if it 
is safe for parallel reading, 
assuming it isn't - please ask 
the extension author to check 
and make it explicit

Is this something worthwhile pursuing for?

@dangotbanned
Copy link
Copy Markdown
Member Author

dangotbanned commented Dec 21, 2024

Regarding feedback on parallel running of sphinx docs:

WARNING: the sphinxext_altair.altairplot 
extension does not declare if it 
is safe for parallel reading, 
assuming it isn't - please ask 
the extension author to check 
and make it explicit

Is this something worthwhile pursuing for?

@mattijn

It has the potential to speed this CI job up, but I think I'd struggle with testing --jobs locally:

This feature only works on systems supporting “fork”. Windows is not supported.

I'm happy to support if anyone else wants to take the lead on it though.
I just imagine getting it working on native posix will be easier than wsl or MINGW64


I really need to transition away from windows at some point 😔
Think it is causing problems in vega/vega-datasets#648 (comment) as well

@mattijn
Copy link
Copy Markdown
Contributor

mattijn commented Dec 21, 2024

Thanks for providing context regarding the environment variable of uv!

In fact I'm quite happy that we have maintainers on multiple operating systems as it increase the robustness of the codebase.

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.

Switch from hatch, pip to uv in GitHub Actions

2 participants