| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
in Debian we also use MATPLOTLIBDATA during matplotlib build, in order to point to a not-yet-installed mpl-data directory, for example: to build the documentation: # build the doc
MPLCONFIGDIR=. MATPLOTLIBDATA=$(CURDIR)/lib/matplotlib/mpl-data/ PYTHONPATH=$(CURDIR)/build/lib.$(PY_PLATFORM)-$(DEFPY) $(MAKE) -C doc html
to run the test suite: -PYTHONPATH=$(shell python$* -c "from distutils.command.build import build ; \
from distutils.core import Distribution ; \
b = build(Distribution()) ; \
b.finalize_options() ; \
print(b.build_platlib)") \
MATPLOTLIBDATA=$(CURDIR)/lib/matplotlib/mpl-data/ \
MPLCONFIGDIR=. \
xvfb-run -a python$* tests.py --no-network --verbose --full-trace
-PYTHONPATH=$(shell python$*-dbg -c "from distutils.command.build import build ; \
from distutils.core import Distribution ; \
b = build(Distribution()) ; \
b.finalize_options() ; \
print(b.build_platlib)") \
MATPLOTLIBDATA=$(CURDIR)/lib/matplotlib/mpl-data/ \
MPLCONFIGDIR=. \
xvfb-run -a python$*-dbg tests.py --no-network --verbose --full-trace
rm -f matplotlibrc
how would we achieve the same result when MATPLOTLIBDATA is gone? |
Sorry, something went wrong.
|
Is installing Matplotlib in a venv and building the docs from there acceptable for Debian? Or apply your mpl-data-moving patch after building the docs. Or patch doc/conf.py to make it apply a patch to matplotlib/__init__.py to find the sample data where needed. |
Sorry, something went wrong.
|
sadly it's not possible to build mpl in a venv and use it to build the documentation (it's just how the debian packaging system works). i understand your suggestions, but i have to make something acceptable by the debian ecosystem, in this case:
if you really want to get rid of MATPLOTLIBDATA, i'll just keep it around as a debian patch, no bid deal |
Sorry, something went wrong.
|
Can you point to me where these rules are listed? Edit: Or perhaps you can patch get_data_path() to add Debian's path as a last fallback. This way, when setting PYTHONPATH to the just-built Matplotlib, it'll use the "normal" path, but when the end-user is importing Matplotlib, the last-fallback path will be used. Or as suggested above you can patch doc/conf.py to do something like import matplotlib; matplotlib.get_data_path = lambda: <the normal data path> |
Sorry, something went wrong.
|
Tests are run against the installed matplotlib in Fedora during build. The reason that MATPLOTLIBDATA is used is because everything is installed in a self-contained destdir. Only specifying the patched path as the fallback only will not work because of this, as we don't really want to include that build root path in the installed version. |
Sorry, something went wrong.
|
Would patching to path = Path(__file__) / "../../(didn't count)/share/data/matplotlib" work? |
Sorry, something went wrong.
|
Actually, though I haven't looked into the history of it, I think the main reason for separating things is to avoid installing twice for Python 2 and 3, but with Matplotlib 3 only supporting Python 3 now... |
Sorry, something went wrong.
The MATPLOTLIBDATA environment variable is only relevant to non-standard
installs (e.g., debian) that move the mpl-data directory to a separate
location (in debian's case, /usr/share/matplotlib/mpl-data); but for
whoever is already patching Matplotlib to achieve this, they may as well
also patch `get_data_path` to always return the new correct path (which
debian already does in their `20_matplotlibrc_path_search_fix.patch`),
reproduced here:
diff --git a/lib/matplotlib/__init__.py b/lib/matplotlib/__init__.py
index 9339707..563b0a8 100644
--- a/lib/matplotlib/__init__.py
+++ b/lib/matplotlib/__init__.py
@@ -738,10 +738,12 @@ def _get_data_path():
return path
_file = _decode_filesystem_path(__file__)
- path = os.sep.join([os.path.dirname(_file), 'mpl-data'])
+ path = '/usr/share/matplotlib/mpl-data'
if os.path.isdir(path):
return path
+ raise RuntimeError('Could not find the matplotlib data files')
+
# setuptools' namespace_packages may highjack this init file
# so need to try something known to be in matplotlib, not basemap
import matplotlib.afm
@@ -836,7 +838,7 @@ def matplotlib_fname():
yield matplotlibrc
yield os.path.join(matplotlibrc, 'matplotlibrc')
yield os.path.join(_get_configdir(), 'matplotlibrc')
- yield os.path.join(get_data_path(), 'matplotlibrc')
+ yield '/etc/matplotlibrc'
for fname in gen_candidates():
if os.path.exists(fname):
|
Re-pinging @QuLogic @sandrotosi after #11241 (comment). Thanks in advance for your input. |
Sorry, something went wrong.
|
I think that is reasonable and would work fine; the self-contained destdir is supposed to mirror the final install exactly after all. |
Sorry, something went wrong.
|
I am -0.5 on this. I don't see much of an upside and see a downside of making the debian packaging a bit more annoying. |
Sorry, something went wrong.
|
The upside is ultimately switching to https://docs.python.org/3/library/importlib.html#module-importlib.resources (Py3.7+) instead of having to bother with _get_data_path at all, and completely remove the rcparam... |
Sorry, something went wrong.
|
FWIW the final patch would make _get_data_path something like (just) global _data_path_cm
_data_path_cm = ExitStack() # This keeps the contextmanager open.
return _data_path_cm.enter_context(
importlib.resources.path('matplotlib', 'mpl-data'))
and not even store it in rcParams (because it doesn't make sense to let the user modify it later anyways -- many things, e.g. the included font paths, are set in stone at import time). |
Sorry, something went wrong.
|
let me just re-state what i said before: i totally understand if you want to get rid of MATPLOTLIBDATA env var to make mpl code easier to maintain. OTOH we kinda like the ability to specify where the mpl-data is at runtime, without patching anything (more than we already do ;) ) or setting a list of "weirdly looking" paths. i'm just fine to come up with a debian-specific patch to support our needs if you ultimately decide to drop MATPLOTLIBDATA |
Sorry, something went wrong.
|
@tacaswell Are you willing to revisit this based on the comments so far? Happy to help with the Debian-specific patch if needed. |
Sorry, something went wrong.
There was a problem hiding this comment.
Ummm, seems fine to me. OTOH I think @tacaswell should merge....
Sorry, something went wrong.
|
@sandrotosi this is coming for 3.1 (next month-ish). |
Sorry, something went wrong.
…241-on-v3.1.x Backport PR #11241 on branch v3.1.x (Deprecate the MATPLOTLIBDATA environment variable.)
|
One minor inconvenience here is that if you're running with a patched path (using Path(__file__).parent ...), then you can't override where an editable install is looking for data. This is something you might do while testing backports, or (in my case), new images for FreeType 2.9.1. |
Sorry, something went wrong.
|
But the baseline images aren't found relative to the data dir? I'm not sure what the issue is, but would be happy to hear more. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
The MATPLOTLIBDATA environment variable is only relevant to non-standard
installs (e.g., debian) that move the mpl-data directory to a separate
location (in debian's case, /usr/share/matplotlib/mpl-data); but for
whoever is already patching Matplotlib to achieve this, they may as well
also patch get_data_path to always return the new correct path (which
debian already does in their 20_matplotlibrc_path_search_fix.patch),
reproduced here:
diff --git a/lib/matplotlib/__init__.py b/lib/matplotlib/__init__.py index 9339707..563b0a8 100644 --- a/lib/matplotlib/__init__.py +++ b/lib/matplotlib/__init__.py @@ -738,10 +738,12 @@ def _get_data_path(): return path _file = _decode_filesystem_path(__file__) - path = os.sep.join([os.path.dirname(_file), 'mpl-data']) + path = '/usr/share/matplotlib/mpl-data' if os.path.isdir(path): return path + raise RuntimeError('Could not find the matplotlib data files') + # setuptools' namespace_packages may highjack this init file # so need to try something known to be in matplotlib, not basemap import matplotlib.afm @@ -836,7 +838,7 @@ def matplotlib_fname(): yield matplotlibrc yield os.path.join(matplotlibrc, 'matplotlibrc') yield os.path.join(_get_configdir(), 'matplotlibrc') - yield os.path.join(get_data_path(), 'matplotlibrc') + yield '/etc/matplotlibrc' for fname in gen_candidates(): if os.path.exists(fname):attn @sandrotosi I guess :)
PR Summary
PR Checklist