-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
maint: convert to cached_property and other minor refactors #1870
base: master
Are you sure you want to change the base?
Conversation
c38af0f
to
a134274
Compare
I messed up |
7370db0
to
bbab731
Compare
Sort imports
Hi @ofek, this is ready for review. Urgency is low. (I just hope this can be merged before too many merge conflicts develop.) I'm trying to refactor to take advantage of recent language features and make other things a little bit simpler to understand. I'm very interested in feedback, especially if you spot any errors. Most commits are independent, so if there's anything you don't like I can rebase it away. Thanks a lot for your consideration, and happy holidays!!! |
This is a continuation of #1760.
I have a few motivations. One is to understand why some of the version-computing logic is so complicated, and to simplify that if possible. (See for example maresb/hatch-vcs-footgun-example#3.) Also, I have fantasies about being able to close #304.
In each case, I need to understand in detail what the code is doing so that I can make modifications without causing breakage. But then I get bogged down in complexity that to me seems inessential. So as I go through this process, I try to either eliminate inessential complexity or to document why the complexity is essential.
Currently, property values are cached by-hand in a variable of the same name prefixed with an underscore. As long as this variable is only used by that property, and only with the usual caching boilerplate, this can be directly converted to
@cached_property
by adding the decorator and removing the cache variable and boilerplate. Sometimes the property is a mutable container likeProjectMetadata.dynamic
(which is depopulated as dynamic values are computed), but this detail doesn't affect the caching logic. In other cases likeCoreMetadata._readme_content_type
, the variable is modified not only by the.readme_content_type
property but also the.readme
property. Since this PR is already getting a bit heavy I have left these cases untouched.Along the way I've made a few other very minor refactors and added some comments.
Hopefully I made no mistakes and these changes are helpful. Thanks!