-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Replace sole use of maxdict by lru_cache. #22323
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
Conversation
:) You can bring the deprecation over here if you want, happy to close the other PR. While you're at it, do you want to add a test here to make sure we are hitting the cache as expected since I made the mistake on the other one that would only cache on each text instance? |
Done (^2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All failures appear to be CI related.
@@ -0,0 +1,4 @@ | |||
``matplotlib.cbook.maxdict`` is deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change name of file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
da764ff
to
44d21ac
Compare
Edit: modified the implementation to not use pickle but instead key off an internally generated copy of fontprop (which the caller cannot access and thus cannot mutate). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modulo one comment improvement. May self-merge after adressing.
Ah, and rebase should fix the CI failure (#22326).
lib/matplotlib/text.py
Outdated
# the passed-in argument do not mess up the cache. dpi is not used, but | ||
# participates in cache invalidation (via the renderer). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# the passed-in argument do not mess up the cache. dpi is not used, but | |
# participates in cache invalidation (via the renderer). | |
# the passed-in argument do not mess up the cache. |
Move that note down, because dpi
is only passed on here. Clever tools will show unused in the function below and people might be tempted to remove it without consulting the context of the call site.
... and expand the cache size, while we're at it.
It was used in one place in the library and there is now a standard library lru_cache that we can take advantage of instead.
... and expand the cache size, while we're at it.
Actual deprecation of maxdict can be done in #22299.
PR Summary
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).