Skip to content

Bug with font finding, and here is my fix as well. #12406

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.

Sign up for GitHub

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

Jump to bottom
Closed
PGTBoos opened this issue Oct 5, 2018 · 7 comments · Fixed by #12408
Closed

Bug with font finding, and here is my fix as well. #12406

PGTBoos opened this issue Oct 5, 2018 · 7 comments · Fixed by #12408
Labels
OS: Microsoft
Milestone
v3.0.1

Comments

@PGTBoos
Copy link

PGTBoos commented Oct 5, 2018 •
edited
Loading

Bug report

font_manager crashes with font not found errors,
Bug summary
there is an Issue allready for this, but after i fixed it i couldnt find that issue back as when i fixed it my errors where gone .

Some error about font not found
Code for reproduction
from matplotlib.font_manager import win32InstalledFonts
print(win32InstalledFonts())

Matplotlib version

  • Operating system: Windows
  • Matplotlib version: matplotlib:2.0.2
  • Matplotlib backend (print(matplotlib.get_backend())):
  • Python version: Python:3.6.2
  • Jupyter version (if applicable): (latest)
  • Other libraries:

Conda default

Here is my fix:

def win32InstalledFonts(directory=None, fontext='ttf'):
"""
Search for fonts in the specified font directory, or use the
system directories if none given.  A list of TrueType font
filenames are returned by default, or AFM fonts if *fontext* ==
'afm'.
"""
import winreg
if directory is None:
    directory = win32FontDirectory()
fontext = get_fontext_synonyms(fontext)
items = set()
for fontdir in MSFontDirectories:
    try:
        with winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, fontdir) as local:
            for j in range(winreg.QueryInfoKey(local)[1]):
                key, direc, tp = winreg.EnumValue(local, j)
                if not isinstance(direc, str):
                    continue
                # Work around for https://bugs.python.org/issue25778, which
                # is fixed in Py>=3.6.1.
                direc = direc.split("\0", 1)[0]
                try:
                    path = Path(directory, direc).resolve()    #errors on file not found
                    items.add(str(path))
                except:
                    #print ("found invalid font direc : {}",direc)
                    continue
    except (OSError, MemoryError):
        continue
return list(items)

Essentially problems are caused because the registry does not really reflect all the fonts, that was the reasons Path(directory,direc).resolve() crashed, so placed it inside another try excerpt and it now works.
The point here is registry does not reflect the file system, people can remove software in wild ways.
i left a commented option to show the missing files in the excerpt.
Its fixed now so i hope other people with the same issues can be happy and go coding now :),
its my first python opensource contribution
@anntzer
Copy link
Contributor

anntzer commented Oct 5, 2018

Calling Path("non/existent/path").resolve() doesn't crash for me; can you show an example of failing case that this fixes?

@PGTBoos
Copy link
Author

PGTBoos commented Oct 5, 2018 •
edited
Loading

notice that in case of the first try: in above code, that the code also didn't provide fonts cause a non existing registrykey 'fontdir' ,
//ps thats probably a cause of error to this is a bad variable name fontdir
//registryfontlist would have been more clear. Its not a file dir itself its a regsitry path.

So fontdir was passed trough (didnt exist in regsitry at all) and that would erase the entire item list.
By doing sub level try catch just before it is checked this is resolved. Besides that the code now checks if fonts truely exist, non existing fonts found in registry are also ignored now. (so that try intercept isnt triggered for the wrong reasons)

some how a wrong registry path was givven suposendly having fonts, well not on my win 10 system the regkey didnt exist., the other key (first in for) did exist though, but because of try catch code in between got ignored (Computer\HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Fonts) didnt exist the other key found by the code did exist, but the whole block of code got ignored (crashed on missing file as well) in the way try catch was setup.

Two errors caused the problems, a wrong registy key, and some font files not existing.
using one try catch for that resulted in empty list, and file not found errors..

@anntzer
Copy link
Contributor

anntzer commented Oct 5, 2018 •
edited
Loading

Ah, I see, this changed in Py3.6:

If the path doesn’t exist and strict is True, FileNotFoundError is raised. If strict is False, the path is resolved as far as possible and any remainder is appended without checking whether it exists. If an infinite loop is encountered along the resolution path, RuntimeError is raised.

New in version 3.6: The strict argument.

in 3.5 this was

̣ If the path doesn’t exist, FileNotFoundError is raised. If an infinite loop is encountered along the resolution path, RuntimeError is raised.

So the fix seems fine but the except clause should specifically just catch FileNotFoundError and RuntimeError (on 3.6, just RuntimeError).

@PGTBoos
Copy link
Author

PGTBoos commented Oct 5, 2018

i'm not coding that long in python just a few days days or so
but i think the code is ok like this, essentially it tries to check a font, on whatever cause it fails will result in not adding it to the list.

English is not my first language, so i hope my bug cause explanation was clear.
And i dont know who puts the fix on git, i dont plan for joining this matplotlib project, could you do that ?
I saw quite a few stackoverflow questions related to this, (also from earlier years).

@anntzer
Copy link
Contributor

anntzer commented Oct 5, 2018

Sure, I'll do it.

@anntzer anntzer mentioned this issue Oct 5, 2018
Merged
6 tasks
@PGTBoos
Copy link
Author

PGTBoos commented Oct 5, 2018

maybe a small side note, that it seamed strange to me that only lowercase font extensions where added in the old code. It doesnt violate windows if WINDINGS2.TTF was used besides windings.ttf

@PGTBoos PGTBoos mentioned this issue Oct 5, 2018
Closed
@anntzer
Copy link
Contributor

anntzer commented Oct 5, 2018

That was already reported and fixed elsewhere.

@QuLogic QuLogic added this to the v3.0.x milestone Oct 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
OS: Microsoft
Projects
None yet
Milestone
v3.0.1
Development

Successfully merging a pull request may close this issue.

Don't crash on invalid registry font entries on Windows. anntzer/matplotlib
3 participants
@QuLogic @anntzer @PGTBoos