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

Add dispatch for --prerelease to uv #811

Closed
wants to merge 2 commits into from

Conversation

cnpryer
Copy link
Contributor

@cnpryer cnpryer commented Feb 28, 2024

Throwing this up just to chat about it more. I only looked at rye add so far.

This is related to #668.

Is there more to this than I realize? Is this a UX problem? If it's a UX problem what about just use clap's default_missing_value and adjust how it's interpreted for when uv isn't enabled and when it is.

rye config --unset behavior.use-uv
rye add <package> --pre
rye config --set-bool behavior.use-uv=true
rye add <package> --pre  # --prerelease if-necessary-or-explicit
rye add <package> --pre allow  # --prerelease allow

@cnpryer
Copy link
Contributor Author

cnpryer commented Feb 28, 2024

So far this comes to mind as a potential problem.

rye add --pre <package>

@mitsuhiko
Copy link
Collaborator

I really do not like if a parameter takes optional args. i think it would be better to say --pre means --prerelease=allow and to have a separate --prerelease parameter.

@cnpryer
Copy link
Contributor Author

cnpryer commented Feb 29, 2024

I really do not like if a parameter takes optional args. i think it would be better to say --pre means --prerelease=allow and to have a separate --prerelease parameter.

Up to you, but maybe there's an alternative we can think of before that. Having --pre and --prerelease both sounds a little awkward.

If the goal is to just move to uv at some point, why don't we just move on from --pre? Offer --prerelease compatibility with original --pre usage by suggesting the use of --prerelease=allow? It's not like we'd want to hang onto --pre if we move on to uv completely, right?

@charliermarsh
Copy link
Member

For what it's worth, in uv we have --prerelease=allow etc., but we just added a hidden --pre alias for pip compatibility that's mutually exclusive with --prerelease and means --prerelease=allow.

@cnpryer
Copy link
Contributor Author

cnpryer commented Feb 29, 2024

For what it's worth, in uv we have --prerelease=allow etc., but we just added a hidden --pre alias for pip compatibility that's mutually exclusive with --prerelease and means --prerelease=allow.

This is smart. I'm happy to do that here.

@cnpryer
Copy link
Contributor Author

cnpryer commented Feb 29, 2024

For reference astral-sh/uv#2049

@mitsuhiko
Copy link
Collaborator

Let's align with uv here.

@cnpryer cnpryer force-pushed the cnp-prerelease branch 2 times, most recently from 7929aca to 835c37b Compare March 3, 2024 03:44
@cnpryer
Copy link
Contributor Author

cnpryer commented Mar 3, 2024

I'm starting to spend more time on this one. In order to finish this I want to understand Rye's locking a little more. I'm not a pip-tools user, so I'll try my best to follow up on a lot of this.

I made this minimum working example to better understand the current state of Rye with --pre usage https://github.com/cnpryer/rye-prerelease-examples.

Here's what I did to set those examples up:

rye-prerelease-examples on  main [?] 
❯ cd with-pip-tools 

rye-prerelease-examples/with-pip-tools on  main [?] is 📦 v0.1.0 via 🐍 v3.12.2 
❯ rye config --unset behavior.use-uv 

rye-prerelease-examples/with-pip-tools on  main [?] is 📦 v0.1.0 via 🐍 v3.12.2 
❯ rye add Django==5.0rc1 --pre            
Added Django==5.0rc1 as regular dependency

rye-prerelease-examples/with-pip-tools on  main is 📦 v0.1.0 via 🐍 v3.12.2 
❯ rye sync
Reusing already existing virtualenv
Generating production lockfile: /Users/chrispryer/github/rye-prerelease-examples/with-pip-tools/requirements.lock
Generating dev lockfile: /Users/chrispryer/github/rye-prerelease-examples/with-pip-tools/requirements-dev.lock
Installing dependencies
Looking in indexes: https://pypi.org/simple/
Obtaining file:///. (from -r /var/folders/x5/94x0_4tj6hz4682vh8l4fv1m0000gn/T/tmp_1qaxysn (line 3))
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Preparing editable metadata (pyproject.toml) ... done
gCollecting asgiref==3.7.2 (from -r /var/folders/x5/94x0_4tj6hz4682vh8l4fv1m0000gn/T/tmp_1qaxysn (line 1))
  Using cached asgiref-3.7.2-py3-none-any.whl.metadata (9.2 kB)
iCollecting django==5.0rc1 (from -r /var/folders/x5/94x0_4tj6hz4682vh8l4fv1m0000gn/T/tmp_1qaxysn (line 2))
  Using cached Django-5.0rc1-py3-none-any.whl.metadata (4.1 kB)
Collecting sqlparse==0.4.4 (from -r /var/folders/x5/94x0_4tj6hz4682vh8l4fv1m0000gn/T/tmp_1qaxysn (line 4))
  Using cached sqlparse-0.4.4-py3-none-any.whl.metadata (4.0 kB)
Using cached asgiref-3.7.2-py3-none-any.whl (24 kB)
Using cached Django-5.0rc1-py3-none-any.whl (8.0 MB)
Using cached sqlparse-0.4.4-py3-none-any.whl (41 kB)
Building wheels for collected packages: with-pip-tools
t  Building editable for with-pip-tools (pyproject.toml) ... done
  Created wheel for with-pip-tools: filename=with_pip_tools-0.1.0-py3-none-any.whl size=1140 sha256=f7430d5956639c448531f6f4ce3c2c3189a874ebfb380829201ec8464fa750cd
  Stored in directory: /private/var/folders/x5/94x0_4tj6hz4682vh8l4fv1m0000gn/T/pip-ephem-wheel-cache-cv35phzh/wheels/8b/19/c8/73a63a20645e0f1ed9aae9dd5d459f0f7ad2332bb27cba6c0f
Successfully built with-pip-tools
Installing collected packages: with-pip-tools, sqlparse, django, asgiref
 status
Successfully installed asgiref-3.7.2 django-5.0rc1 sqlparse-0.4.4 with-pip-tools-0.1.0
Done!
rye-prerelease-examples on  main [?] is 📦 v0.1.0 via 🐍 v3.12.2 
❯ rye config --set-bool behavior.use-uv=true

rye-prerelease-examples on  main [?] 
❯ cd with-uv 

rye-prerelease-examples/with-uv on  main [?] is 📦 v0.1.0 via 🐍 v3.12.2 
❯ rye add Django==5.0rc1 --pre              
Added django>=5.0rc1 as regular dependency
Reusing already existing virtualenv
Generating production lockfile: /Users/chrispryer/github/rye-prerelease-examples/with-uv/requirements.lock
Generating dev lockfile: /Users/chrispryer/github/rye-prerelease-examples/with-uv/requirements-dev.lock
Installing dependencies
   Built file:///Users/chrispryer/github/rye-prerelease-examples/with-uv                                                       Built 1 editable in 169ms
Installed 4 packages in 44ms
 + asgiref==3.7.2
 + django==5.0.2
 + sqlparse==0.4.4
 + with-uv==0.1.0 (from file:///Users/chrispryer/github/rye-prerelease-examples/with-uv)
Done!
rye-prerelease-examples on  main is 📦 v0.1.0 via 🐍 v3.12.2 
❯ rye --version
rye 0.28.0
commit: 0.27.0+14 (37f5ba9b7 2024-03-02)
platform: macos (aarch64)
self-python: [email protected]
symlink support: true

Note that that only the pip-tools method includes the Django release candidate in the lock file. Both pyproject.tomls look fine at first glance.

It's also worth noting that neither example includes this in the lockfiles.

# last locked with the following flags:
#   pre: true

Here are the venv packages:

image

I'm also curious of if we are actually applying this on a per-dependency basis, or if during fresh syncs it's only able to apply it to the entire sync.

@mitsuhiko
Copy link
Collaborator

The reading back of the lock comments would still have to be honored. Likewise the legacy pre: true flag would have to updated. Look at LockOptions::restore for what I mean.

@cnpryer
Copy link
Contributor Author

cnpryer commented Mar 5, 2024

The reading back of the lock comments would still have to be honored. Likewise the legacy pre: true flag would have to updated. Look at LockOptions::restore for what I mean.

Thanks for the comment, but I understand this part. What I was looking at was how rye add <pkg> --pre would propagate pre: true into the lockfile. From this example I don't believe it does that.

I'd assume the idea originally, from a UX perspective, was to use rye sync --pre after staging dependency (using pip-tools version of rye add). So what I'm trying to get to is understanding if there's (1) actually an issue there or if I'm just misunderstanding something, and (2) if the there is an issue with that I'm wondering if I should try and scoop it up here.

I'm leaning towards following up on that later.

@cnpryer
Copy link
Contributor Author

cnpryer commented Mar 8, 2024

I'm going to close this for now. I won't be able to get back to this until the 15th.

@cnpryer cnpryer closed this Mar 8, 2024
@cnpryer cnpryer deleted the cnp-prerelease branch March 8, 2024 22:09
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.

3 participants