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

feat: allow for editable pypi source dependencies when using path #1044

Merged
merged 26 commits into from
Mar 28, 2024

Conversation

tdejager
Copy link
Contributor

@tdejager tdejager commented Mar 22, 2024

This PR adds the ability to do editable installs for pypi-packages. This requires the project to also use a pyproject.toml so that we know what build system to use.

This PR takes great inspiration on how UV does it with some changes.

  1. We resolve editables for multiple platforms by using the metadata separately.
  2. We only* actually build the wheel on install time.
  3. There is some deviating installation logic because we can install directly from locked dependencies.

I think that when this PR lands you mostly want to use editable installs for path based dependencies that you control, instead of regular path based dependencies.

Locking

For locking we actually lock whether the dependency is editable and also save a hash to the pyproject.toml, setup.cfg, or setup.py files. If any of these change, the lock file is invalidated, this is analogous to how UV does this for determining whether to re-install but their code uses the timestamp instead.

@tdejager tdejager marked this pull request as draft March 22, 2024 15:30
@tdejager tdejager marked this pull request as ready for review March 26, 2024 14:52
Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

Nice work!! It worked for some project I have on my machine. Lets share it with the world.

Some small remarks but in general I approve!

Comment on lines +185 to +202
# pep440_rs = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# pep508_rs = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-build = { git = "https://github.com/wolfv/uv", tag = "expose-yanks" }
# uv-cache = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-client = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-dispatch = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-distribution = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-installer = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-interpreter = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-normalize = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-resolver = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-traits = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# distribution-filename = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# distribution-types = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# install-wheel-rs = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# platform-tags = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# pypi-types = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# requirements-txt = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# pep440_rs = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# pep508_rs = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-build = { git = "https://github.com/wolfv/uv", tag = "expose-yanks" }
# uv-cache = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-client = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-dispatch = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-distribution = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-installer = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-interpreter = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-normalize = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-resolver = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# uv-traits = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# distribution-filename = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# distribution-types = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# install-wheel-rs = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# platform-tags = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# pypi-types = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }
# requirements-txt = { git = "https://github.com/wolfv/uv", branch = "expose-yanks" }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No we need to keep this because we have to patch it very often.

let installed = site_packages.iter().filter(|dist| {
dist.installer()
.unwrap_or_default()
.is_some_and(|installer| installer == "uv")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still uv or should this also be pixi-uv?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That PR is still open :)

@ruben-arts ruben-arts merged commit 36ac560 into prefix-dev:main Mar 28, 2024
19 of 20 checks passed
@henryiii
Copy link
Contributor

henryiii commented Apr 3, 2024

This doesn't seem to be doing what it's supposed to do. If I add the following to particle, for example:

[tool.pixi.project]
name = "particle"
channels = ["conda-forge"]
platforms = ["osx-64"]

[tool.pixi.dependencies]
particle  = { path = ".", editable = true}

Then it does not do an editable install, and it does not install from the local path. Instead, it pulls particle from conda-forge. If the name in tool.pixi.dependencies is not valid from conda-forge, it errors. I do get this warning:

 WARN pixi::project::manifest: BETA feature `[pypi-dependencies]` enabled!

@henryiii
Copy link
Contributor

henryiii commented Apr 3, 2024

Ahh, I bet I got the wrong section name. Yep, [tool.pixi.pypi-dependencies] works. It would have been nice if editable/path was an error for conda dependencies.

@tdejager
Copy link
Contributor Author

tdejager commented Apr 4, 2024

Indeed, let's make an issue out of that :)

@@ -12,6 +12,9 @@ python = ">=3.12"
flask = ">=3.0.2,<3.1"
gunicorn = ">=21.2.0,<21.3"

[pypi-dependencies]
docker-project = { path = ".", editable = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't this clash with postinstall-production below @tdejager?

Copy link
Contributor Author

@tdejager tdejager Apr 4, 2024

Choose a reason for hiding this comment

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

Hmm yes it will get overwriten probably. But the .pth files might incorrectly stay in the prefix. We need the #1092 to do it correctly.

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.

5 participants