Skip to content

Conversation

@oscarkey
Copy link
Contributor

@oscarkey oscarkey commented Oct 2, 2025

Previously we used our own script to generate the minimum dependencies to test the compatibility of our package. This doesn't support environment markers, which is blocking my next PR, and has also previously caused issues:
534d4c3#diff-64a02d4ff986c9ed55bfeb17ee7f2882087cd60211ddcce8836459941bb95bf9
uv now has this functionality built in, so replace our script with that.

I select "lowest-direct", because "lowest" also chooses the minimum versions of transitive dependencies, and these often have no minimum version so we get very old versions. This also matches the behaviour of the current script.

Also add minimum versions to the dev dependencies, which makes "lowest-direct" resolution work nicely when testing locally.

This changes the names of the required checks. My plan to force-merge this PR, then change the names of the required checks to the new ones.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies dependency management by removing a custom script and leveraging uv's built-in resolution capabilities. This is a positive change that reduces maintenance overhead. The accompanying update to pyproject.toml to add minimum versions for development dependencies is also a good practice that supports this change. However, a critical oversight is that the test file corresponding to the deleted script, tests/test_scripts.py, has not been removed. This will break the test suite and needs to be addressed.

Previously we used our own script to generate the minimum dependencies.
This sometimes went wrong
e.g. 534d4c3#diff-64a02d4ff986c9ed55bfeb17ee7f2882087cd60211ddcce8836459941bb95bf9
and it turns out uv has this functionality built in, so use that.

I select "lowest-direct", because "lowest" also chooses the minimum
versions of transitive dependencies, and these often have no minimum
version so we get a very old version.

Also add minimum versions to the dev dependencies, which makes
"lowest-direct" resolution work nicely when testing locally.
@oscarkey oscarkey requested a review from LeoGrin October 6, 2025 08:51
Copy link
Collaborator

@LeoGrin LeoGrin left a comment

Choose a reason for hiding this comment

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

Oh that's cool!

@oscarkey oscarkey merged commit 125162f into main Oct 6, 2025
10 checks passed
@oscarkey oscarkey deleted the ok-uv-dep-spec branch October 6, 2025 09:11
oscarkey added a commit that referenced this pull request Nov 12, 2025
…nimum dependency set. (#173)

* Record copied public PR 535

* Switch to uv's "--resolution" option to install minimum dependency set. (#535)

Previously we used our own script to generate the minimum dependencies to test the compatibility of our package. This doesn't support environment markers, which is blocking my next PR, and has also previously caused issues:
534d4c3#diff-64a02d4ff986c9ed55bfeb17ee7f2882087cd60211ddcce8836459941bb95bf9
uv now has this functionality built in, so replace our script with that.

I select "lowest-direct", because "lowest" also chooses the minimum versions of transitive dependencies, and these often have no minimum version so we get very old versions. This also matches the behaviour of the current script.

Also add minimum versions to the dev dependencies, which makes "lowest-direct" resolution work nicely when testing locally.

This changes the names of the required checks. My plan to force-merge this PR, then change the names of the required checks to the new ones.

---------

Co-authored-by: mirror-bot <mirror-bot@users.noreply.github.com>
Co-authored-by: Oscar Key <oscar@priorlabs.ai>
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