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

Unify editable and unnamed URL parsing #3946

Merged
merged 1 commit into from
May 31, 2024
Merged

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented May 31, 2024

Summary

This will help prevent bugs like #3934 by unifying the implementations for editables and non-editable unnamed requirements. Specifically, both of these now go through the same parsing paths and use the same struct representations (with the exception that the editable flag is flipped in the first case):

-e ./foo/bar
./foo/bar

We also now support PEP 508 in editable URLs. It turns out this is just a limitation in pip, so it's correct to support it. For example, this now works:

-e black[d] @ file://${PROJECT_ROOT}/scripts/packages/black_editable

Closes #3941.

Closes #3942.

@charliermarsh charliermarsh force-pushed the charlie/editable-url branch 2 times, most recently from 10ba857 to d29071f Compare May 31, 2024 20:15
@charliermarsh charliermarsh added bug Something isn't working enhancement New feature or request labels May 31, 2024
@charliermarsh charliermarsh marked this pull request as ready for review May 31, 2024 20:16
@charliermarsh charliermarsh force-pushed the charlie/editable-url branch 2 times, most recently from 53ef785 to c53d89f Compare May 31, 2024 20:20
@charliermarsh charliermarsh force-pushed the charlie/editable-url branch 3 times, most recently from 70f9660 to 7be1377 Compare May 31, 2024 20:51
Copy link

codspeed-hq bot commented May 31, 2024

CodSpeed Performance Report

Merging #3946 will improve performances by 6.5%

Comparing charlie/editable-url (8146a88) with main (8c11f99)

Summary

⚡ 1 improvements
✅ 12 untouched benchmarks

Benchmarks breakdown

Benchmark main charlie/editable-url Change
wheelname_tag_compatibility[flyte-short-incompatible] 955.8 ns 897.5 ns +6.5%

@charliermarsh
Copy link
Member Author

Deleted a few hundred lines of code but added a few hundred in tests and snapshots.

@charliermarsh charliermarsh enabled auto-merge (squash) May 31, 2024 21:03
@charliermarsh charliermarsh merged commit 7b7da80 into main May 31, 2024
46 checks passed
@charliermarsh charliermarsh deleted the charlie/editable-url branch May 31, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deduplicate unnamed URL and editable URL parsing Support PEP 508-style editable requirements
2 participants