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

fix: incorrect python version was used in some parts of the solve #2481

Merged

Conversation

tdejager
Copy link
Contributor

@tdejager tdejager commented Nov 14, 2024

This fixes the over-use of the Intepreter in the solve, it was using it incorrectly in some parts of the resolution. This would result in weird behavior, like getting the incorrect wheels from the registry and not being to able to match them with the wheel tags, for example.

Discovered because of: #771 (comment). This should now work.

Test

To test you can try:

[project]
channels = ["conda-forge"]
name = "test"
platforms = ["osx-arm64", "linux-64"]

[dependencies]
scipy = ">=0.19"

[pypi-dependencies]
finch-mlir = ">=0.0.2"

Which did not work before and now it does.

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.

Appart from the warning, this looks good to me!

We should quickly get this in a release.

src/lock_file/update.rs Outdated Show resolved Hide resolved
@ruben-arts ruben-arts enabled auto-merge (squash) November 14, 2024 15:24
Copy link
Contributor

@hameerabbasi hameerabbasi left a comment

Choose a reason for hiding this comment

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

Typo, plus maybe we should add tests?

src/lock_file/resolve/pypi.rs Outdated Show resolved Hide resolved
@nichmor
Copy link
Contributor

nichmor commented Nov 14, 2024

Looks good! I think we can add this example in our test_integration to make sure that we will not introduce this regression again

@ruben-arts ruben-arts merged commit e393c66 into prefix-dev:main Nov 15, 2024
40 checks passed
tdejager added a commit that referenced this pull request Nov 15, 2024
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