Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
FIX: Catch IOError on font-cache write#9676
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Do not error on font-cache write error
It is not worth finding a way to force this exception to test it. |
lgtm |
Backport PR#9676 on branch v2.1.x
| try: | ||
| json.dump(data, fh, cls=JSONEncoder, indent=2) | ||
| except IOError as e: | ||
| warnings.warn('Could not save font_manager cache ', e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Guys, this use of warnings.warn does not seem to match the API specification:
https://docs.python.org/2/library/warnings.html#warnings.warn
I was just hit by this problem:
Traceback (most recent call last):File "/my/python/file.py", line 15, in <module> from matplotlib import pyplot as pltFile "/usr/lib64/python2.7/site-packages/matplotlib/pyplot.py", line 31, in <module> import matplotlib.colorbarFile "/usr/lib64/python2.7/site-packages/matplotlib/colorbar.py", line 36, in <module> import matplotlib.contour as contourFile "/usr/lib64/python2.7/site-packages/matplotlib/contour.py", line 20, in <module> import matplotlib.font_manager as font_managerFile "/usr/lib64/python2.7/site-packages/matplotlib/font_manager.py", line 1469, in <module> _rebuild()File "/usr/lib64/python2.7/site-packages/matplotlib/font_manager.py", line 1454, in _rebuild json_dump(fontManager, _fmcache)File "/usr/lib64/python2.7/site-packages/matplotlib/font_manager.py", line 979, in json_dump warnings.warn('Could not save font_manager cache ', e)TypeError: issubclass() arg 1 must be a classThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It doesn't seem to match the py3 version as well:
https://docs.python.org/3/library/warnings.html#warnings.warn
The function's 2nd argument takes an exception class, not an exception instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@jklymak Can you take a look at this? If you want me to send a patch, no problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ooops, well, at least its not worse than it was. A PR would be great, or if you aren't comfortable I could add one. Its pretty hard to test unless you get this error somehow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Belay that - this code is not in master anymore, so it'll be fixed next release. Sorry about the inconveninece in this release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's not inmaster, but it is in 2.2.2, so that may not be the next release. Though I suppose it technically might not count as a regression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah I guess we could backport a fix onto 2.2.3 if there is one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There will definitely be 2.2.3, 2.2 is going to keep getting bug-fix releases until 2020.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It'd be no problem for me to send a PR. It's just that, at the time, I had a lot of other things to do so I just commented out here. Since you guys already have the repositories cloned and everything, it was the quicker solution.
Next time I'll create a PR. Thanks!
Uh oh!
There was an error while loading.Please reload this page.
Fails gracefully if font-manager cache cannot be written.
Fixes#9548
PR Summary
PR Checklist