Skip to content

Use uv in Makefile and CI #1546

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

Merged
merged 41 commits into from
Nov 21, 2024
Merged

Use uv in Makefile and CI #1546

merged 41 commits into from
Nov 21, 2024

Conversation

sydney-runkle
Copy link
Contributor

Follow up to #1545

@sydney-runkle sydney-runkle changed the title Use uv in Makefile Use uv in Makefile and CI Nov 19, 2024
@sydney-runkle
Copy link
Contributor Author

Putting a pause on this for now, will resume tomorrow

@davidhewitt davidhewitt added the Full Build cause CI to do a full build label Nov 20, 2024
@sydney-runkle
Copy link
Contributor Author

I don't think we should need those .txt files with the updates to pyproject.toml - I was using the install groups successfully 👍

@sydney-runkle
Copy link
Contributor Author

I think the codspeed fix might be that we need to add back the install python

Copy link

codspeed-hq bot commented Nov 20, 2024

CodSpeed Performance Report

Merging #1546 will degrade performances by 12.27%

Comparing makefile-uv (d940668) with main (c9d01c3)

Summary

❌ 1 regressions
✅ 154 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main makefile-uv Change
test_list_of_strs_json_uncached 408.2 µs 465.3 µs -12.27%

Copy link
Contributor Author

@sydney-runkle sydney-runkle left a comment

Choose a reason for hiding this comment

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

Maybe we should do something like pydantic/pydantic#10901?

Are we caching things correctly? Is the uv cache ever being used?

@davidhewitt
Copy link
Contributor

🎉

@sydney-runkle, if you are happy with this being good enough, let's merge and iterate any small improvements in follow ups.

@sydney-runkle
Copy link
Contributor Author

Yep, let's merge. Thanks for finishing this up @davidhewitt

@sydney-runkle sydney-runkle merged commit 1e5e899 into main Nov 21, 2024
62 of 63 checks passed
@sydney-runkle sydney-runkle deleted the makefile-uv branch November 21, 2024 14:19
@samuelcolvin
Copy link
Member

@sydney-runkle @davidhewitt did you try the uv run maturin develop commands in this PR?

They seem to always fail for me on another project:

📦 Including license file "/Users/samuel/code/rtoml/LICENSE"
🍹 Building a mixed python/rust project
🔗 Found pyo3 bindings
🐍 Found CPython 3.11 at /Users/samuel/code/rtoml/.venv/bin/python
📡 Using build options features, bindings from pyproject.toml
   Compiling pyo3-build-config v0.21.2
   Compiling pyo3-macros-backend v0.21.2
   Compiling pyo3-ffi v0.21.2
   Compiling pyo3 v0.21.2
   Compiling pyo3-macros v0.21.2
   Compiling rtoml v0.11.0 (/Users/samuel/code/rtoml)
    Finished `release` profile [optimized] target(s) in 6.69s
📦 Built wheel for CPython 3.11 to /var/folders/s6/mh0x1z6d6mqclzdtv72zgx_40000gn/T/.tmpES6rcu/rtoml-0.11.0-cp311-cp311-macosx_11_0_arm64.whl
💥 maturin failed
  Caused by: pip install in /Users/samuel/code/rtoml/.venv failed running ["-m", "pip", "--disable-pip-version-check", "install", "--no-deps", "--force-reinstall", "/var/folders/s6/mh0x1z6d6mqclzdtv72zgx_40000gn/T/.tmpES6rcu/rtoml-0.11.0-cp311-cp311-macosx_11_0_arm64.whl"]: exit status: 1
--- Stdout:

--- Stderr:
/Users/samuel/code/rtoml/.venv/bin/python: No module named pip
---

@davidhewitt
Copy link
Contributor

Think it needs to be uv run maturin develop --uv (unless pip is installed in the environment by uv pip install pip)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Full Build cause CI to do a full build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants