-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: maintain return type stability in win32InstalledFonts #12174
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@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.... |
Just tested this change locally, yes it does fix the import issue. |
Not sure if this should be addressed here, or another PR. |
@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 |
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 |
Apologies for claiming a hang. It turns out if you have a large tree under your current directory, matplotlib/lib/matplotlib/font_manager.py Lines 126 to 133 in 1a40221
will get used in this glob **/*.* matplotlib/lib/matplotlib/font_manager.py Lines 150 to 158 in 1a40221
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. |
Looks like this was a typo, and then a flake commit included |
@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. |
opened #12176 |
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
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.
@Cory-Kramer in the code here, at line 209, can you replace |
Another issue is if the former L204 had raised, I suspect that the change is the removal of the inner most (I suspect @anntzer is already a step of ahead of me as I write this comment ;) ) |
not really... |
The only way that can result in |
I am happy to defer to @Kojoley on this issue. |
No matter when or why that code path is executed, the return value should be |
Superseeded by #12213 |
closes #12173