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

Use uv for CI #311

Merged
merged 19 commits into from
Mar 12, 2024
Merged

Use uv for CI #311

merged 19 commits into from
Mar 12, 2024

Conversation

ejm714
Copy link
Collaborator

@ejm714 ejm714 commented Mar 1, 2024

Swaps in drivendataorg/setup-python-uv-action and installs our dependencies with uv (without caching).

Closes #312

Copy link

netlify bot commented Mar 1, 2024

Deploy Preview for silly-keller-664934 ready!

Name Link
🔨 Latest commit be58fb6
🔍 Latest deploy log https://app.netlify.com/sites/silly-keller-664934/deploys/65ef68f61764c10008799982
😎 Deploy Preview https://deploy-preview-311--silly-keller-664934.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

github-actions bot commented Mar 1, 2024

.github/workflows/tests.yml Outdated Show resolved Hide resolved
@jayqi
Copy link
Member

jayqi commented Mar 2, 2024

I think densepose fails to install because they do a build-time check of whether torch is installed, which doesn't work with modern standard of isolated builds. It looks like uv doesn't currently support installing without build isolation: astral-sh/uv#1715

(The discussion in the thread is also a good summary explaining the history and the why of packages behaving like densepose.)

@jayqi
Copy link
Member

jayqi commented Mar 2, 2024

@ejm714 I just thought of a possible gotcha in how uv and setup-python-uv-action is set up. It's possible that the virtual environment created by setup-python-uv-action doesn't have pip installed in it, and so when you run pip install densepose, it's using the system pip. You might want to print out which pip and/or run uv pip install pip before trying to install densepose.

@jayqi
Copy link
Member

jayqi commented Mar 8, 2024

uv 0.1.16, released yesterday, now supports --no-build-isolation so I think you should be able to use it to install densepose.

https://github.com/astral-sh/uv/releases/tag/0.1.16

Copy link

codecov bot commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.6%. Comparing base (dc87a8a) to head (be58fb6).

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #311   +/-   ##
======================================
  Coverage    87.6%   87.6%           
======================================
  Files          26      26           
  Lines        2178    2178           
======================================
  Hits         1908    1908           
  Misses        270     270           

@ejm714
Copy link
Collaborator Author

ejm714 commented Mar 11, 2024

In the end, looks like a 5-10 min speed up which is small in a relative sense. Tests are still the slowest part of this and we stick to pip for the installation tests (due to transitive dependencies) so overall run is still at least 30 min.

Ready for a final look @pjbull or @jayqi

@ejm714 ejm714 requested review from pjbull and jayqi March 11, 2024 21:04
Copy link
Member

@pjbull pjbull 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, thanks for tackling!

@pjbull pjbull merged commit de0df1e into master Mar 12, 2024
15 checks passed
@pjbull pjbull deleted the uv-for-ci branch March 12, 2024 02:57
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.

Failed build on master branch (tests #672)
3 participants