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

preserver marker order at union #383

Merged
merged 1 commit into from
May 30, 2022

Conversation

dimbleby
Copy link
Contributor

@dimbleby dimbleby commented May 30, 2022

per python-poetry/poetry#5721, use of a set made the representation of markers non-deterministic.

A set is really the appropriate data structure, and this change is a bit of an uglification. But the number of elements in a marker union is very rarely going to be more than, say, 5 - so I doubt that efficiency is an issue.

An alternative that I considered was to try and impose an ordering in the __str__, but:

  • markers don't order
  • could do it alphabetically, but that's going to cause significantly more damage to existing unit tests than I would like to have to deal with...

I also decided that unit test wasn't very useful: a regression in the exact same way doesn't seem very likely. Can add the testcase from python-poetry/poetry#5721 if anyone thinks it's worth the trouble

@sonarcloud
Copy link

sonarcloud bot commented May 30, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@neersighted neersighted merged commit e016bd3 into python-poetry:main May 30, 2022
@dimbleby dimbleby deleted the preserve-marker-order branch May 30, 2022 20:43
@radoering radoering mentioned this pull request Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants