Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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 to44d21acCompareEdit: 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). |
timhoffm left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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, becausedpi 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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
... and expand the cache size, while we're at it.
It was used in one place in the library and there is nowa 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
pytestpasses).flake8-docstringsand 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).