FIX: maintain return type stability in win32InstalledFonts

[tacaswell]

closes #12173

• Has Pytest style unit tests
• Code is Flake 8 compliant
• New features are documented, with examples if plot related
• Documentation is sphinx and numpydoc compliant
• Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
• Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

[tacaswell]

@Cory-Kramer can you see if this fixes it? I don't have a windows machine I can test on or know how to exercise this code path on appveyor....

[Cory-Kramer]

Just tested this change locally, yes it does fix the import issue.

[dsentinel]

Not sure if this should be addressed here, or another PR.
Similar issue on mac details in #12173

[jklymak]

@dsentinel I'm unable to reproduce on master with my mac, so not sure whats going on here. Any chance you are up to checking this PR on your setup? Its a bit of a heavy ask because you need to add @tacaswell 's repo as a remote and then pip install -e . , but once you learn how to do it, its not too bad.

[dsentinel]

I'd be happy to test the tree, but not sure I will be able to today

If you want to try with a conda my environment.yml

name: mat_bug_py37
channels:
  - conda-forge
  - defaults
dependencies:
  - ca-certificates=2018.8.24=ha4d7672_0
  - certifi=2018.4.16=py37_0
  - clangdev=6.0.1=default_1
  - cycler=0.10.0=py_1
  - freetype=2.9.1=h6debe1e_4
  - icu=58.2=hfc679d8_0
  - kiwisolver=1.0.1=py37h2d50403_2
  - libcxx=6.0.1=0
  - libffi=3.2.1=hfc679d8_5
  - libiconv=1.15=h470a237_3
  - libpng=1.6.35=ha92aebf_2
  - libxml2=2.9.8=h422b904_5
  - llvm-meta=6.0.1=0
  - llvmdev=6.0.1=hf8ce74a_2
  - matplotlib=3.0.0=py37h45c993b_0
  - ncurses=6.1=hfc679d8_1
  - openssl=1.0.2p=h470a237_0
  - pip=18.0=py37_1
  - pyparsing=2.2.1=py_0
  - python=3.7.0=h4eca856_1
  - python-dateutil=2.7.3=py_0
  - pytz=2018.5=py_0
  - readline=7.0=haf1bffa_1
  - setuptools=40.4.0=py37_0
  - six=1.11.0=py37_1
  - sqlite=3.25.1=hb1c47c0_0
  - tk=8.6.8=ha92aebf_0
  - tornado=5.1=py37h470a237_1
  - wheel=0.31.1=py37_1
  - xz=5.2.4=h470a237_1
  - zlib=1.2.11=h470a237_3
  - blas=1.0=mkl
  - intel-openmp=2019.0=118
  - libgfortran=3.0.1=h93005f0_2
  - mkl=2019.0=118
  - mkl_fft=1.0.4=py37h5d10147_1
  - mkl_random=1.0.1=py37h5d10147_1
  - numpy=1.15.1=py37h6a91979_0
  - numpy-base=1.15.1=py37h8a80b8c_0

don't forget the bug shows if ~/.matplotlib/fontlist-v300.json does not exist.

[dsentinel]

Apologies for claiming a hang. It turns out if you have a large tree under your current directory,
The empty string at the end of this list:

matplotlib/lib/matplotlib/font_manager.py

Lines 126 to 133 in 1a40221

	OSXFontDirectories = [
	"/Library/Fonts/",
	"/Network/Library/Fonts/",
	"/System/Library/Fonts/",
	# fonts installed via MacPorts
	"/opt/local/share/fonts",
	"",
	]

will get used in this glob **/*.*

matplotlib/lib/matplotlib/font_manager.py

Lines 150 to 158 in 1a40221

	def list_fonts(directory, extensions):
	"""
	Return a list of all fonts matching any of the extensions, found
	recursively under the directory.
	"""
	extensions = ["." + ext for ext in extensions]
	return [str(path)
	for path in filter(Path.is_file, Path(directory).glob("**/*.*"))
	if path.suffix in extensions]

and this can take a long while... minutes+ but will eventually return, if your FS is finite.

Not very pleasant, but still probably a bug.

Hard to reproduce if you're in a small dir trying to recreate :/ ... These hairy bugs are a pain, almost eluded me.

[dsentinel]

Looks like this was a typo, and then a flake commit included "" which pathlib interperetes as .
https://github.com/matplotlib/matplotlib/pull/11963/files#r218979454

[tacaswell]

@dsentinel can you open a new issue for that, taking a long time to find all of the paths is a different issue that this one.

[dsentinel]

opened #12176

[anntzer]

The patch looks fine but I'm a bit confused why we didn't hit this before. On the one hand, in 898998c#diff-915a89f5ee0062db3d8190c8505a240fL186 there is the slightly ominous deletion of

        if not local:
            return list_fonts(directory, fontext)

which is the only relevant difference I can find. On the other hand, the docs clearly say that any non-closed, non-detached key is truthy (https://docs.python.org/3/library/winreg.html#registry-handle-objects) and that failure to open a key results in an exception, not a falsy key (https://docs.python.org/3/library/winreg.html#winreg.OpenKey), so it's not clear how that specific conditional could ever have been triggered before.

I think this needs a bit more investigation and my quick fix may not be right. Dismissing the review instead of adding my own because I can not add my own review to my PR.

[anntzer]

@Cory-Kramer in the code here, at line 209, can you replace continue by import traceback; traceback.print_exc(); continue and let us know what comes out?

[tacaswell]

Another issue is if the former L204 had raised, local would have not been defined and the if not local would have raise a NameError.

I suspect that the change is the removal of the inner most try...except block. Maybe something in the inner most for-loop is raising an OSError or MemoryError which is in turn skipping over the return at tho bottom of the with block and jumping to the end?

(I suspect @anntzer is already a step of ahead of me as I write this comment ;) )

[anntzer]

not really...

[Kojoley]

The only way that can result in None after 898998c#diff-915a89f5ee0062db3d8190c8505a240fL186 change is if winreg.EnumValue throws an exception for some reason (f.ex. number of entries in registry has lowered since winreg.QueryInfoKey call during the loop).

[tacaswell]

I am happy to defer to @Kojoley on this issue.

[timhoffm]

No matter when or why that code path is executed, the return value should be [] instead of None. It's simply a matter of API consistency.

[Kojoley]

Superseeded by #12213