Skip to content

Change win32InstalledFonts return value - #12213

Merged
tacaswell merged 1 commit into
matplotlib:masterfrom
timhoffm:win32InstalledFonts
Sep 23, 2018
Merged

tacaswell merged 1 commit into
matplotlib:masterfrom
timhoffm:win32InstalledFonts

Conversation

@timhoffm

@timhoffm timhoffm commented Sep 22, 2018 •

Copy link
Copy Markdown
Member

PR Summary

In the context of #12173 it was noted that matplotlib.font_manager.win32InstalledFonts() returned None if no fonts are found. This is inconsistent with the docs which claims that a list of fonts is returned. This PR changes the return value to an empty list, which is a more natural choice and is more convenient than the None value. If the value can be None, we have to explicitly check before doing any list-operations on the return value. An empty list can be handled more gracefully.

Returning an empty list is also the behavior of OSXInstalledFonts().

This is in principle a breaking API change. However, I think it's ok to document it and apply it right away.

Note: this fixes one of the exceptions in #12173:

  File "C:\Python\Python35\lib\site-packages\matplotlib\font_manager.py", line 264, in findSystemFonts
    fontfiles.update(win32InstalledFonts(fontext=fontext))
TypeError: 'NoneType' object is not iterable

PR Checklist

  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@timhoffm timhoffm added this to the v3.0.x milestone Sep 22, 2018
@timhoffm timhoffm added the API: changes Changes to the public API, typically requiring deprecation. label Sep 22, 2018
@Kojoley

Kojoley commented Sep 22, 2018

Copy link
Copy Markdown
Member

There is already #12174, however it did not add an api change entry.

@timhoffm

Copy link
Copy Markdown
Member Author

Oh, didn't see #12174. The discussion there has gone a bit off-topic as well.

Anyway, we should add an API change note. While it's unlikely that someone will actually use that function and have a problem with the change, it's still a function that appears in the official docs.

@Kojoley Kojoley mentioned this pull request Sep 22, 2018
@timhoffm

Copy link
Copy Markdown
Member Author

Taking up the solution proposed by @Kojoley in #12173 (comment)

@Lyghtning

Copy link
Copy Markdown

I don't know if this is allowed or not, I'm a complete novice when it comes to Python, and libraries, imports. What I can say is I had this problem, ended up here after having to do an environment reload which updated my matplotlib.

You guys are amazing, checked out the file update, made the modifications, back up and running.
Thank you for being such contributor to newbies like me. Thank you seriously.

@sebbosafened

sebbosafened commented Sep 26, 2018 •

Copy link
Copy Markdown

The solution proposed by @Kojoley in #12173 did not solve the issue for me, I still get

 File "C:\Users\thisuser\AppData\Local\Programs\Python\Python35\lib\site-packages\matplotlib\font_manager.py", line 264, in findSystemFonts
    fontfiles.update(win32InstalledFonts(fontext=fontext))
TypeError: 'NoneType' object is not iterable

@Kojoley

Kojoley commented Sep 26, 2018

Copy link
Copy Markdown
Member

@sebbosafened Hm, I do not know how it is even possible to get a None value after the PR.

line 264

Please, check twice, the line number should be different.

fontfiles.update(win32InstalledFonts(fontext=fontext))

@sebbosafened

sebbosafened commented Sep 27, 2018 •

Copy link
Copy Markdown

I copied everything from your version of font_manager.py (

fontfiles.update(win32InstalledFonts(fontext=fontext))
) to my local font_manager.py (after backing up my local version that I had already edited - with obvious mistakes - according to yourt post @Kojoley on 23rd september, 00:16 CEST) and now it works. Thank you a lot.

@zunction

Copy link
Copy Markdown

I copied everything from your version of font_manager.py (

matplotlib/lib/matplotlib/font_manager.py

Line 261 in deae70a

fontfiles.update(win32InstalledFonts(fontext=fontext))
) to my local font_manager.py (after backing up my local version that I had already edited - with obvious mistakes - according to yourt post @Kojoley on 23rd september, 00:16 CEST) and now it works. Thank you a lot.

Somehow the contents of font_manager.py have to be manually updated like what @sebbosafened did. I also managed to get it to work after updating my local version of font_manager.py to the version here. Thanks a lot for solving the problem 👍

@AndrewPanB

Copy link
Copy Markdown

I copied everything from your version of font_manager.py (
matplotlib/lib/matplotlib/font_manager.py
Line 261 in deae70a
fontfiles.update(win32InstalledFonts(fontext=fontext))
) to my local font_manager.py (after backing up my local version that I had already edited - with obvious mistakes - according to yourt post @Kojoley on 23rd september, 00:16 CEST) and now it works. Thank you a lot.

Somehow the contents of font_manager.py have to be manually updated like what @sebbosafened did. I also managed to get it to work after updating my local version of font_manager.py to the version here. Thanks a lot for solving the problem

I change "if sys.platform == 'win32:" to if sys.platform == 'win64': in matplotlib/lib/matplotlib/font_manager.py
It works.

@Kojoley

Kojoley commented Oct 9, 2018

Copy link
Copy Markdown
Member

I change "if sys.platform == 'win32:" to if sys.platform == 'win64': in matplotlib/lib/matplotlib/font_manager.py

It cannot has that value https://mail.python.org/pipermail/patches/2000-May/000648.html
https://github.com/python/cpython/blob/c510c6b8b60f211793e0b84c317ea6974e8a6153/PC/pyconfig.h#L297-L301

@jckling

jckling commented Oct 10, 2018

Copy link
Copy Markdown

I copied everything from your version of font_manager.py (
matplotlib/lib/matplotlib/font_manager.py
Line 261 in deae70a
fontfiles.update(win32InstalledFonts(fontext=fontext))
) to my local font_manager.py (after backing up my local version that I had already edited - with obvious mistakes - according to yourt post @Kojoley on 23rd september, 00:16 CEST) and now it works. Thank you a lot.

Somehow the contents of font_manager.py have to be manually updated like what @sebbosafened did. I also managed to get it to work after updating my local version of font_manager.py to the version here. Thanks a lot for solving the problem

I change "if sys.platform == 'win32:" to if sys.platform == 'win64': in matplotlib/lib/matplotlib/font_manager.py
It works.

It works for me! Thanks a lot 🎉🎉🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API: changes Changes to the public API, typically requiring deprecation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants