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

Caching properties to deduplicate sources #5863

Merged
merged 1 commit into from
Aug 26, 2023

Conversation

kalebmckale
Copy link
Contributor

@kalebmckale kalebmckale commented Aug 23, 2023

Without caching, the Resolver duplicates the sources listed in the Pipfile multiple times (since the property re-adds them every time they're called) in the arguments sent to the pip parser.

Removing unused command-line option --debug

Thank you for contributing to Pipenv!

The issue

While diagnosing another issue, I found through print debugging that the multiple calls to these properties, pip_args and pip_options, ended up duplicating the sources listed in the Pipfile multiple times for the pip parser.

Related to the resolver, the --debug command-line option for pipenv-resolver is unused.

The fix

To fix this, I replaced property decorator with cached_property, which was already imported in the module. As best as I can tell, these properties (and a number of other ones) only need to be evaluated once instead of every single time they are called. Caching not only fixes this duplication but will also lead to faster, more efficient code.

If a debug option is later implemented, the command-line option can be added at that time.

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

Without caching, the Resolver will duplicate the sources listed in the
`Pipfile` multiple times (since the property re-adds them every
they're called) in the arguments sent to the `pip` parser.

Removing unused command-line option `--debug`
@kalebmckale
Copy link
Contributor Author

@matteius What type of change would this be considered for my news file?

@matteius
Copy link
Member

@kalebmckale I'd be fine with bugfix.

@matteius matteius marked this pull request as ready for review August 26, 2023 13:36
@matteius
Copy link
Member

I am going to include a news fragment on your behalf so we can include this fix in the release.

@matteius matteius merged commit ea79c0d into pypa:main Aug 26, 2023
19 checks passed
@kalebmckale
Copy link
Contributor Author

Thanks!

@kalebmckale kalebmckale deleted the cache-dedupe-sources branch August 26, 2023 22:58
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.

2 participants