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

Use lockfile versions as resolution preferences #3921

Merged
merged 2 commits into from
May 30, 2024

Conversation

charliermarsh
Copy link
Member

Summary

Ensures that we avoid upgrading packages unless --upgrade or similar is passed.

For now, the resolver only respects these for registry distributions.

Closes #3918.

@charliermarsh charliermarsh marked this pull request as ready for review May 30, 2024 01:46
@charliermarsh charliermarsh added the preview Experimental behavior label May 30, 2024
@@ -52,6 +52,11 @@ impl Lock {
Lock::try_from(wire)
}

/// Returns the [`Distribution`] entries in this lock.
pub fn distributions(&self) -> &[Distribution] {
Copy link
Member Author

Choose a reason for hiding this comment

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

@BurntSushi - Let me know if you have a preferred way to do this (other than what's here). I expose Distribution, but none of the fields or methods are pub.

Copy link
Member

Choose a reason for hiding this comment

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

I think this looks fine.

The thing I care about most is not exposing representation. I don't mean to say we shouldn't do it, but that's where the biggest benefit is. If we don't expose representation, then it makes it much easier to change that representation in the future. (It's all a trade off.)

Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

This is awesome!

@@ -52,6 +52,11 @@ impl Lock {
Lock::try_from(wire)
}

/// Returns the [`Distribution`] entries in this lock.
pub fn distributions(&self) -> &[Distribution] {
Copy link
Member

Choose a reason for hiding this comment

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

I think this looks fine.

The thing I care about most is not exposing representation. I don't mean to say we shouldn't do it, but that's where the biggest benefit is. If we don't expose representation, then it makes it much easier to change that representation in the future. (It's all a trade off.)

@charliermarsh charliermarsh changed the base branch from konsti/move-requirement-to-pypi-types to main May 30, 2024 17:40
@charliermarsh charliermarsh enabled auto-merge (squash) May 30, 2024 17:40
Copy link

codspeed-hq bot commented May 30, 2024

CodSpeed Performance Report

Merging #3921 will degrade performances by 6.17%

Comparing charlie/preferences (d7f803b) with main (502e042)

Summary

⚡ 1 improvements
❌ 1 regressions
✅ 11 untouched benchmarks

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

Benchmarks breakdown

Benchmark main charlie/preferences Change
resolve_warm_airflow 1.5 s 1.4 s +7.48%
resolve_warm_jupyter 80.2 ms 85.5 ms -6.17%

@charliermarsh charliermarsh merged commit 1445669 into main May 30, 2024
45 of 46 checks passed
@charliermarsh charliermarsh deleted the charlie/preferences branch May 30, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uv lock should respect preferences from existing lockfile
2 participants