Workaround for illegal package merging in py-matplotlib/py-basemap by citibeth · Pull Request #1964 · spack/spack

citibeth · 2016-10-07T18:57:19Z

This PR addresses #1948

Matplotlib defines a Python package called mpl_tools. They put some Python code in that package, so it does not qualify as a Python Implicit Namespace Package ( https://www.python.org/dev/peps/pep-0420/ ).

Basemap also defines mpl_tools, implicitly as an extension of Matplotlib's mpl_tools. The Python package mpl_tools can now be found twice on $PYTHONPATH: once in the py-mathplotlib Spack package, and once in the py-basemap Spack package. This is not legal Python, except for the special case of an implicit namespace package (which does not apply here).

Maybe some Python interpreter in the past implicitly "merged" the two mpl_tools directories together, and it is clear that the authors of Matplotlib/Basemap added Jujitsu in those directories to try to make it happen that way (see the __init__.py files). HOWEVER... it does not currently work with MY versions of Matplotlib (1.5.1), Basemap (1.0.7) and Python (3.5.2) on Spack. On MY system, Python sees only the basemap version of mpl_tools. It then throws an exception when Basemap tries to import things that are in the matplotlib version of mpl_tools.

The right way to solve this problem is to re-work Basemap and Matplotlib so they don't step on each others' toes that way. It's not clear how this would be done, since this bad design decision is now baked into the library APIs. This problem has been an issue for years without getting fixed; see for example (From 2014): https://github.com/Homebrew/homebrew-python/issues/112

This PR solves the problem by symlinking all of the mpl_tools directory from matplotlib into basemap. It's an un-holy mess, and wouldn't scale if there were additional packages that also implicitly "merge" into mpl_tools (there are none that I know of). But it solves the problem at hand, it doesn't break the Spack paridigm, and is better than any other solution I could think of; for example, to create a Spack package that installs Matplotlib and Basemap simultaneously into one Spack prefix.

# Conflicts: # var/spack/repos/builtin/packages/py-basemap/package.py

adamjstewart · 2016-10-07T19:09:30Z

var/spack/repos/builtin/packages/py-basemap/package.py

+        # legal Python for "Implicit Namespace Packages":
+        #     https://www.python.org/dev/peps/pep-0420/
+        #     https://github.com/Homebrew/homebrew-python/issues/112
+        # In practice, Python will wee only the basemap version of mpl_toolkits


typo: wee -> see

tgamblin · 2016-10-11T09:36:37Z

@lee218llnl can you do a quick test of this?

citibeth · 2016-10-11T14:56:59Z

My only misgiving is that it used to work for me without this fix and now it doesn't, and I still don't know why. I've already ruled out issues related to the version of Matplotlib or Basemap itself.

Looking in the Python changelog, I see that starting with Python 3.3, they support namespace packages in a standardized way:
https://docs.python.org/3/whatsnew/3.3.html#pep-420-implicit-namespace-packages
https://www.python.org/dev/peps/pep-0420/#namespace-packages-today
Matplotlib and Basemap don't use this, of course, they use the pre-3.3 non-standardized way to split packages across directories.

I also notice that Python 3.5.2 came into Spack this July; before that, I was using Python 3.5.1. And it did work then (I know it didn't work for my August build).

My best guess is that Python 3.5.2 made a change that broke the way Basemap and Matplotlib split packages across directories --- because they do it in a non-standard way. I think the right approach to this is:

Add `when='@3.5.2:' to my fix in Spack; if we find the fix is needed for earlier Pythons, we can change it later.
Ask upstream to follow PEP-420 when building with Python 3.3 or greater. Once this is done, we can remove my fix from the Spack package.

….5.2

lee218llnl · 2016-10-14T19:13:47Z

var/spack/repos/builtin/packages/py-basemap/package.py

+        # We are not sure if this fix is needed before Python 3.5.2.
+        # If it is needed, this test should be changed.
+        # See: https://github.com/LLNL/spack/pull/1964
+        if spec.version >= Version('3.5.2'):


Won't spec.version refer to basemap's version?

citibeth · 2016-10-14T19:24:35Z

Thanks, good catch

On Fri, Oct 14, 2016 at 3:13 PM, Gregory Lee [email protected]
wrote:

@lee218llnl commented on this pull request.

In var/spack/repos/builtin/packages/py-basemap/package.py:
 depends_on('pil', type=nolink)
 depends_on("geos")
 def install(self, spec, prefix):
     env['GEOS_DIR'] = spec['geos'].prefix
     setup_py('install', '--prefix=%s' % prefix)
   # We are not sure if this fix is needed before Python 3.5.2.
   # If it is needed, this test should be changed.
   # See: https://github.com/LLNL/spack/pull/1964
   if spec.version >= Version('3.5.2'):
Won't spec.version refer to basemap's version?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1964 (review), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd9t0sereAMlIFDIoMHSm88t6V-bGks5qz9RtgaJpZM4KRWY5
.

lee218llnl · 2016-10-14T20:46:47Z

No problem, this looks good to me now. I can merge it if you think it is ready @citibeth.

citibeth · 2016-10-14T22:23:21Z

Yes
On Oct 14, 2016 4:46 PM, "Gregory Lee" [email protected] wrote:

No problem, this looks good to me now. I can merge it if you think it is
ready @citibeth https://github.com/citibeth.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1964 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1cd9vyC4pcPmUj3XFRlQ2KmI4umfrkks5qz-o6gaJpZM4KRWY5
.

…pack#1964) * Workaround for illegal package merging in py-matplotlib/py-basemap # Conflicts: # var/spack/repos/builtin/packages/py-basemap/package.py * flake8 * flake8 * Be conservative: only apply the namespace package fix for Python >= 3.5.2 * flake8 * Bug fix

Workaround for illegal package merging in py-matplotlib/py-basemap

c0163ef

# Conflicts: # var/spack/repos/builtin/packages/py-basemap/package.py

citibeth added the bug Something isn't working label Oct 7, 2016

This was referenced Oct 7, 2016

Make mpl_toolkits a non-namespace package matplotlib/matplotlib#5327

Closed

py-matplotlib: Taming the dependencies #1965

Closed

adamjstewart reviewed Oct 7, 2016

View reviewed changes

Elizabeth Fischer added 2 commits October 7, 2016 15:14

flake8

c6f0264

flake8

6bfdbde

Elizabeth Fischer added 2 commits October 11, 2016 11:03

Be conservative: only apply the namespace package fix for Python >= 3…

229c6ce

….5.2

flake8

99de1e1

lee218llnl reviewed Oct 14, 2016

View reviewed changes

Bug fix

069e6f2

tgamblin added python extensions labels Oct 14, 2016

tgamblin merged commit 3dbde09 into spack:develop Oct 14, 2016

citibeth mentioned this pull request Feb 28, 2017

spack view needs to merge easy-install.pth files #3245

Closed

citibeth mentioned this pull request Mar 28, 2017

Don't build eggs. #3587

Merged

adamjstewart mentioned this pull request Apr 12, 2017

mpl_toolkits.basemap fails to import #3813

Closed

lee218llnl mentioned this pull request Apr 14, 2017

matplotlib and basemap require setuptools to run properly together #3835

Merged

citibeth mentioned this pull request Jul 16, 2018

py-basemap broken again #8716

Closed

citibeth pushed a commit to citibeth/spack that referenced this pull request Sep 25, 2018

Redo spack#1964; see spack#8716

e3c5fa5

citibeth mentioned this pull request Apr 1, 2019

py-basemap: Re-apply fix #10590

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Workaround for illegal package merging in py-matplotlib/py-basemap#1964

Workaround for illegal package merging in py-matplotlib/py-basemap#1964
tgamblin merged 6 commits intospack:developfrom
citibeth:efischer/161007-FixBasemapSplitPackage

citibeth commented Oct 7, 2016

Uh oh!

adamjstewart Oct 7, 2016

Uh oh!

tgamblin commented Oct 11, 2016

Uh oh!

citibeth commented Oct 11, 2016

Uh oh!

lee218llnl Oct 14, 2016

Uh oh!

citibeth commented Oct 14, 2016

@lee218llnl commented on this pull request.

Uh oh!

lee218llnl commented Oct 14, 2016

Uh oh!

citibeth commented Oct 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

citibeth commented Oct 7, 2016

Uh oh!

adamjstewart Oct 7, 2016

Choose a reason for hiding this comment

Uh oh!

tgamblin commented Oct 11, 2016

Uh oh!

citibeth commented Oct 11, 2016

Uh oh!

lee218llnl Oct 14, 2016

Choose a reason for hiding this comment

Uh oh!

citibeth commented Oct 14, 2016

@lee218llnl commented on this pull request.

Uh oh!

lee218llnl commented Oct 14, 2016

Uh oh!

citibeth commented Oct 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants