FIX: ensure we import when the user cwd does not exist #19507
Conversation
|
This was reported to me via private channels as a fatal "application won't start issue". |
lib/matplotlib/__init__.py
Outdated
| # as the current working directory do not fail to import. | ||
| yield os.path.join(os.getcwd(), 'matplotlibrc') | ||
| except FileNotFoundError: | ||
| pass |
There was a problem hiding this comment.
I think I'd feel better if a log message was emitted. People may be wondering why their matplotlibrc file isn't getting picked up.
There was a problem hiding this comment.
what matplotlibrc? $cwd already doesn't exist, so $cwd/matplotlibrc certainly doesn't either?
There was a problem hiding this comment.
Also, it may be easier to just return a relative path here (return "matplotlibrc") and then absolutize it at exit (if os.path.exists(fname) and not os.path.isdir(fname): return os.path.abspath(fname)), which would avoid the exception-checking and also absolutize something set from $MATPLOTLIBRC (well, I'm not sure we ever detailed the semantics of relative-path $MATPLOTLIBRC...)
There was a problem hiding this comment.
@anntzer, I am referring to a hypothetical matplotlibrc file that could have been in the cwd(), but is no longer there because the user didn't realize that they are running matplotlib in a directory that has been removed (an extension on the scenario described by @tacaswell). A log message here could provide a useful breadcrumb for a user to come to this realization. The use of relative paths as you'd suggested would actually paper over this problem and make it harder for a user to realize what is going on.
There was a problem hiding this comment.
I'm not sure I follow. The same case would apply to a hypothetical matplotlibrc that's in ~/.config/matplotlib/matplotlibrc and that somehow got deleted before you import matplotlib; we don't warn in that case either (i.e. if there's no ~/.config/matplotlib/matplotlibrc, or ~/.config/matplotlib for that matter).
There was a problem hiding this comment.
If we go this route I don't think we need to abspath it. These paths go to open in very short order we can let it take care of relative paths.
@WeatherGod I'm with @anntzer here, unless we warn when we don't find a local matplotlibrc (but with the folder existing) I don't think we should warn when we do not find a local matplotlibrc (because $cwd does not exist).
This is the only place that we directly access the cwd in the main library.
46a8343 to
962e609
Compare
|
Went with the simpler approach (rather than catching |
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
|
I'll let @QuLogic decide whether this should be 3.4.0 or 3.4.1. |
PR Summary
This is the only place that we directly access the cwd in the main library.
You can drive your self to this state by in one shell cd'ing to a directory and then from a different shell removing that directory. The first shell will still think it's working directory is the (now non-existent) path and mild chaos ensues.
This has probably been broken forever (at least back to the 2.2.x series), however I do not think that it is worth the trouble to set up this weird state in the tests. If you are in this state IPyhon will not start and PyQt5 will not import.
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).