NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.4k

Python 3.9 upgrade#24980

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

Jump to bottom

Merged

ksunden merged 3 commits intomatplotlib:mainfromgreglucas:py39-upgrade

Jan 21, 2023

Merged

Python 3.9 upgrade#24980

ksunden merged 3 commits intomatplotlib:mainfromgreglucas:py39-upgrade

Jan 21, 2023

Conversation

Copy link

Contributor

greglucas commentedJan 14, 2023

PR Summary

Now that we are on Python 3.9+ (#24919), we can update/simplify some of the code.

Automated command line utility for it:

pyupgrade --py39-plus `find . -name "*.py" -type f`

A lot of this is f-string updates, so I added the first commit to the git ignore blame so there is less churn in the blame lookups.
There are also some OSError aliases and getting rid of the unnecessary parentheses in thelru_cache decorator

PR Checklist

Documentation and Tests

[N/A] Has pytest style unit tests (andpytest passes)
[N/A] Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
[N/A] New plotting related features are documented with examples.

Release Notes

[N/A] New features are marked with a.. versionadded:: directive in the docstring and documented indoc/users/next_whats_new/
[N/A] API changes are marked with a.. versionchanged:: directive in the docstring and documented indoc/api/next_api_changes/
[N/A] Release notes conform with instructions innext_whats_new/README.rst ornext_api_changes/README.rst

Copy link

ContributorAuthor

greglucas commentedJan 14, 2023

pre-commit.ci autofix

ksunden reviewed

Jan 14, 2023

View reviewed changes

examples/text_labels_and_annotations/arrow_demo.py OutdatedShow resolvedHide resolved

ksunden reviewed

Jan 14, 2023

View reviewed changes

lib/matplotlib/backends/backend_pgf.py OutdatedShow resolvedHide resolved

ksunden reviewed

Jan 14, 2023

View reviewed changes

lib/matplotlib/backends/backend_svg.py OutdatedShow resolvedHide resolved

ksunden reviewed

Jan 14, 2023

View reviewed changes

lib/matplotlib/ticker.py OutdatedShow resolvedHide resolved

ksunden reviewed

Jan 14, 2023

View reviewed changes

lib/matplotlib/tests/test_backend_bases.pyShow resolvedHide resolved

oscargus reviewed

Jan 14, 2023

View reviewed changes

lib/matplotlib/backends/backend_ps.py Outdated

		"/{name}{{{bbox} sc\n".format(
		name=font.get_glyph_name(glyph_id),
		bbox=" ".join(map(str, [g.horiAdvance, 0, *g.bbox])),
		)

Copy link

Member

oscargusJan 14, 2023

Choose a reason for hiding this comment

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

Ithink that one can get f-strings here as well (although those will be horrible...) if it is executed again?

oscargus reviewed

Jan 14, 2023

View reviewed changes

lib/matplotlib/backends/backend_ps.py OutdatedShow resolvedHide resolved

Copy link

Member

oscargus commentedJan 14, 2023•
edited
Loading

In general, I'm tempting to skip the f-string conversion and let it evolve "organically" as some of them are quite painful as@ksunden points out (others are obviously a clear improvement). Also, some that are turned into format should probably be f-strings (others not).

(Interesting that some unpacking comprehensions are converted into tuples, but not things likeif a in ["b", "c"]:.)

Edit: to be clear, things that are not f-string conversions, I do not object to.

anntzer reviewed

Jan 14, 2023

View reviewed changes

lib/matplotlib/__init__.py

		@@ -231,7 +231,7 @@ def _check_versions():

		# The decorator ensures this always returns the same handler (and it is only
		# attached once).
		@functools.lru_cache()

Copy link

Contributor

anntzerJan 14, 2023

Choose a reason for hiding this comment

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

Some of these should be converted to functions.cache for clarity, in fact.

Copy link

ContributorAuthor

greglucasJan 17, 2023

Choose a reason for hiding this comment

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

I updated some of them where I thought an unbounded cache was alright, but others I figured we can handle later if we want to change to unbounded rather than the default size of 128. When I was looking through thelru_cache locations it looked like we may even have some decorated instance methods with self that may be keeping instances alive, I wonder if that is causing any of the slowly growing memory leaks that have been reported...

Copy link

Contributor

anntzerJan 17, 2023

Choose a reason for hiding this comment

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

I was specifically referring to cases where the cache just serves to establish a singleton, like here.

anntzer reviewed

Jan 14, 2023

View reviewed changes

lib/matplotlib/_docstring.py OutdatedShow resolvedHide resolved

greglucas force-pushed thepy39-upgrade branch from42e6fd5 to3f8e632Compare

January 17, 2023 03:31

Copy link

ContributorAuthor

greglucas commentedJan 17, 2023

I went through every.format() call and subjectively evaluated whether f-strings would be nicer or not. So, this is definitely most of the locations that can/should be updated now (even more than the automatic formatter picked up the first go-around). I personally prefer to just get formatting changes updated in one go so you can ignore that commit more easily through blames.

greglucas commented

Jan 17, 2023

View reviewed changes

lib/matplotlib/lines.py OutdatedShow resolvedHide resolved

greglucas force-pushed thepy39-upgrade branch 2 times, most recently from89c996d to5c9d97eCompare

January 17, 2023 04:44

Copy link

Member

timhoffm commentedJan 18, 2023

I think it would be a good idea to add a.git-blame-ignore-revs file. See alsohttps://www.stefanjudis.com/today-i-learned/how-to-exclude-commits-from-git-blame/

Copy link

ContributorAuthor

greglucas commentedJan 18, 2023

I think it would be a good idea to add a .git-blame-ignore-revs file. See alsohttps://www.stefanjudis.com/today-i-learned/how-to-exclude-commits-from-git-blame/

Yes, I agree and we already have one that I updated here :)
https://github.com/matplotlib/matplotlib/pull/24980/files#diff-f247fbfbe928b907db42554d0b3006b28dd11c25a59be031abda73b11a2c934a

ksunden previously approved these changes

Jan 19, 2023

View reviewed changes

ksunden added this to thev3.8.0 milestone

Jan 19, 2023

QuLogic reviewed

Jan 20, 2023

View reviewed changes

Copy link

Member

QuLogic left a comment

Choose a reason for hiding this comment

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

This is good; I can drop my f-string stash now.

examples/lines_bars_and_markers/bar_label_demo.py OutdatedShow resolvedHide resolved

lib/matplotlib/_mathtext.py OutdatedShow resolvedHide resolved

lib/matplotlib/backends/backend_ps.pyShow resolvedHide resolved

lib/matplotlib/cbook.py OutdatedShow resolvedHide resolved

lib/matplotlib/dviread.py OutdatedShow resolvedHide resolved

lib/matplotlib/tri/_trirefine.py OutdatedShow resolvedHide resolved

setup.py OutdatedShow resolvedHide resolved

tools/github_stats.py OutdatedShow resolvedHide resolved

tools/gh_api.py OutdatedShow resolvedHide resolved

greglucas force-pushed thepy39-upgrade branch from5c9d97e to3e720e4Compare

January 20, 2023 02:42

Copy link

ContributorAuthor

greglucas commentedJan 20, 2023

Thanks,@QuLogic, good recommendations as usual!

ksunden dismissed theirstale review

January 20, 2023 18:06

conversations in setup.py comments

greglucas added3 commits

January 20, 2023 19:14

MNT: Update code with Python 3.9+ syntax

4d30640

Command: `pyupgrade --py39-plus`Manual updates to add some f-strings and ignore f-strings in texformats.

MNT: Add git blame ignore for pyupgrade 3.9+

ffbb4c1

MNT: Update lru_cache to cache where appropriate

63a8ad4

greglucas force-pushed thepy39-upgrade branch from3e720e4 to63a8ad4Compare

January 21, 2023 02:16

QuLogic approved these changes

Jan 21, 2023

View reviewed changes

ksunden merged commit6a9a071 intomatplotlib:main

Jan 21, 2023

greglucas deleted the py39-upgrade branch

January 21, 2023 16:15

ksunden mentioned this pull request

Jan 25, 2023

MAINT: don't format logs in log call.#25073

Merged

6 tasks

anntzer mentioned this pull request

Feb 5, 2025

Use functools.cache instead of lru_cache to establish singletons.#29581

Merged

5 tasks

Labels

None yet

6 participants

Movatterモバイル変換

Uh oh!

Python 3.9 upgrade#24980

Python 3.9 upgrade#24980

Uh oh!

Conversation

greglucas commentedJan 14, 2023

PR Summary

PR Checklist

Uh oh!

greglucas commentedJan 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oscargusJan 14, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

oscargus commentedJan 14, 2023• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

anntzerJan 14, 2023

Choose a reason for hiding this comment

Uh oh!

greglucasJan 17, 2023

Choose a reason for hiding this comment

Uh oh!

anntzerJan 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greglucas commentedJan 17, 2023

Uh oh!

Uh oh!

timhoffm commentedJan 18, 2023

Uh oh!

greglucas commentedJan 18, 2023

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greglucas commentedJan 20, 2023

Uh oh!

Uh oh!

oscargus commentedJan 14, 2023•
edited
Loading