Conversation
dpranke
left a comment
There was a problem hiding this comment.
Thanks for the PR! This seems fine assuming the CI jobs all still pass ... let's find out :).
|
Oh, wait, I misread the diff ... the changes to ci.yml shouldn't cause or require the changes to uv.lock, right? Can we just change ci.yml and leave uv.lock unchanged? If we need to update uv.lock as part of adding support for 3.14, let's do that in a separate PR (perhaps one of the others you've uploaded?), if we can. |
revoking approval at least temporarily per my other comments ... I'll be happy to approve a revised patch.
Not a hard requirement and doesn't cause a failure but I'd still say "they kinda do". The reason I updated the lock file is that the new version of the uv uses revision 3, while the existing lock file is revision 1. That being said, I can create a separate PR to update the lock file using the current uv version, then here in this PR lock again with the new uv version to this PR won't include any unrelated changes. It looks uv can't update the revision without running a full update. |
|
I just updated the PR to separate actual updates from the revision change. If the new versions looks better, I can separate the first commit into its own PR. If it still feels noisy, I can create a PR for the uv update and separate that change from 3rd party action updates. |
|
I see. What version of uv are you using to generate the latest lock file? Can we change pyproject.toml to use that version w/o triggering a bunch of other diffs and w/o dropping support for old versions (we might need to add version conditionals on uv to do that). It looks like the version in pyproject.toml is pretty old at this point (as presumably are the other versions, of course). |
|
Oh I completely missed the one in the pyproject.toml file. I was using 0.9.18, and considering this is a package and we won't want to pin to a patch version, I'd recommend going with One question tho, why uv is needed as a project dependency? People will need the uv installed to be able to interact with those dependencies in the first place, so I'm missing where it will be used in the project itself. If the goal is to keep pyproject.toml and uv versions compatible, I'll recommend using |
|
I agree that we should probably add |
I support the intention, but this doesn't seem to work with uv. I tried to install an older version for #96 (comment) and it looks like the system's uv is still prioritized. Using a different version requires a different step - uv in the active venv is not used: |
|
Interesting. I'll have to take a look later when I have more time to play with things. I'll assume you're right in the meantime :). However, documenting which version of uv is actually used seems useful, and doing it here seems perhaps as good as anywhere else. Maybe add a comment that the version isn't actually used? Or even leave the version, but comment it out (along with a comment as to why it's commented out)? I'm open to alternatives, too. |
|
whoops, ignore the "ready to review" bit, I hit the wrong button by mistake :). |
|
I think staying in the same revision should be enough to create reproducible builds/environments, and |
|
Good to know. |
|
Okay, playing with things a bit to follow up on your comment ... I was seeing the same thing with uv versions too, and was thoroughly befuddled until I dug into it. I'm not sure what you were seeing, but I am guessing it was one of two things:
E.g., running on a Mac, using bash v5.3.9(1), I was seeing something like: Which made little sense to me. But, then I remembered that bash caches $PATH lookups, and so if you invoke a binary via the normal $PATH search (rather than specifying an explicit path), and then add a new binary somewhere in your $PATH before the directory containing the previous one, bash may not automatically see and use it. You can check this with: (in the same environment we left off ...) So, once i fixed that, it seemed like I picked up the "correct" uv from the virtual environment. Can you check to see if either of these things explains what you were seeing? That said, learning more now, it does seem like using |
I'm using zsh and testing the same steps but they don't change the result.
All 👍🏻 Would you be interested in using the following pattern, so that Renovate can generate automatic dependency updates? |
I'm not familiar w/ renovate (apart from what I've learned in looking at it just now). I had been thinking I needed to adopt something like dependabot, and it looks like renovate is similar, so, sure :). Do you have particular things you like about renovate more? |
Hmm. And you're sure you activated the venv? I get the same thing in zsh 5.9 as I get in bash, except that after I've added uv to the venv and it should be getting picked up, I get just At any rate, I'm happy to move forward and remove the entry as discussed. |
The goal is to refresh the content, and minimize the noise for other PRs that will be doing changes in pyprpoject.toml. Coming from the discussion under dpranke#96, We could live without an "lock --upgrade" for removing uv dependency, but updating the revision in the lock file requires "lock --upgrade". Running an inital upgrade with the existing uv version helps to separate actual package updates from revision related changes.
The goal is to refresh the content, and minimize the noise for other PRs that will be doing changes in `pyproject.toml. Coming from the discussion under dpranke#96, We could live without an "lock --upgrade" for removing uv dependency, but updating the revision in the lock file requires "lock --upgrade". Running an inital upgrade with the existing uv version helps to separate actual package updates from revision related changes.
dpranke
left a comment
There was a problem hiding this comment.
Sorry for the delay :(. Looks good!
* Update lock file The goal is housekeeping. Having an updated lock file before applying changes to packages feels better. * Convert uv dependency to "uv.required-version" setting Following up #96 (comment) * Update uv version
|
@dpranke Coming from #96 (comment), Renovate does better what Dependabot claims it does.
|
Cool, I'll take a look, thanks. I'm working on publishing a new version 0.13.0 with your changes and updated dependencies now. |
|
I was planning to create a PR for Python 3.14 support but I realized that CI dependencies are outdated.