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

perf: let Transaction.calculate_operations run in O(N) #8063

Merged
merged 2 commits into from
Dec 1, 2024

Conversation

ralbertazzi
Copy link
Contributor

Hard to call it a perf commit, as we are talking about a few milliseconds, but I think it also makes the code more Pythonic

@Secrus Secrus added this to the Poetry 2.0 milestone Oct 6, 2024
@radoering radoering force-pushed the perf/transaction-dict branch from 22b36eb to b01d3a2 Compare December 1, 2024 11:17
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

Rebased on current main with some minor adaptions:
I only adopted the change for _installed_packages because that is the only list without duplicate package names. Building a dict of _current_packages and _result_packages is dangerous because it drops distinct entries with the same name. Further, there is no need for a dict for _current_packages and (with changes on the main branch, which happened in the meantime) there is no need for a dict for _result_packages anymore.

I agree that making _installed_packages a dict improves maintainability.

@radoering radoering merged commit cdfd955 into python-poetry:main Dec 1, 2024
73 checks passed
Copy link

github-actions bot commented Jan 1, 2025

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants