Skip to content
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

Warn if a temporary config/cache dir must be created. #15933

Merged
merged 1 commit into from May 1, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Dec 13, 2019

Paying the cost of regen'ing the font cache on every import seems a bit
silly when one can just set MPLCONFIGDIR to a persistent directory.
Moreover the tmpdir approach is brittle against forking; this is fixable
but somewhat complex to do.

Closes #15911, see also #832 (see discussions in both of them).
Edit: also closes #16677.

PR Summary

PR Checklist

  • 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

lib/matplotlib/__init__.py Outdated Show resolved Hide resolved
@anntzer anntzer force-pushed the warntmpconfigdir branch 2 times, most recently from a19d9c5 to 5837012 Compare December 15, 2019 16:10
@timhoffm timhoffm added this to the v3.3.0 milestone Dec 15, 2019
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a test that the warning is being thrown, which I guess is possible by setting 'MPLCONFIGDIR' to a path that can't be made

@anntzer
Copy link
Contributor Author

anntzer commented Dec 17, 2019

Ah, good catch re: testability, didn't think of that possibility, done.

@anntzer
Copy link
Contributor Author

anntzer commented Jan 17, 2020

looks like chmod() doesn't work that easily on Windows (https://docs.python.org/3/library/os.html#os.chmod); possibly fixable but I'm going to claim that a non-writable home directory is sufficiently uncommon on Windows that we can skip the test there (and in any case the actual functionality should still work there).

@anntzer
Copy link
Contributor Author

anntzer commented Mar 5, 2020

@dstansby good to go?

Paying the cost of regen'ing the font cache on every import seems a bit
silly when one can just set MPLCONFIGDIR to a persistent directory.
Moreover the tmpdir approach is brittle against forking; this is fixable
but somewhat complex to do.
@anntzer
Copy link
Contributor Author

anntzer commented Apr 1, 2020

rebased

@QuLogic QuLogic dismissed dstansby’s stale review May 1, 2020 00:01

There's a test.

@QuLogic QuLogic merged commit 7f808e7 into matplotlib:master May 1, 2020
@anntzer anntzer deleted the warntmpconfigdir branch May 1, 2020 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tmp_config_or_cache_dir atexit cleanup fails after forks()
5 participants