Skip to content

ENH: ship our own copy of six 1.9.0 - #4501

Closed
matthew-brett wants to merge 1 commit into
matplotlib:masterfrom
matthew-brett:ship-six-1.9.0
Closed

matthew-brett wants to merge 1 commit into
matplotlib:masterfrom
matthew-brett:ship-six-1.9.0

Conversation

@matthew-brett

Copy link
Copy Markdown
Contributor

Six seems to be the external dependency that is most likely to cause problems,
and it is a small standalone .py file that is designed for projects to ship in
their source tree.

Shipping has the following advantages:

  • Reduces pip dependencies by one;
  • Makes mpl less vulnerable to upstream changes in six. At present, it
    is possible for six to change upstream in pypi and break matplotlib
    installs, requiring a new mpl release.

It does mean that we don't get the lastest six, but I believe this is an
advantage for the reason above. Now we are so far into the Python 3 era, six
should be rather stable.

Projects like Debian can be aggressive about separating dependencies, but in
this case, I believe this would be severe overkill - six is specifically
designed for shipping inside other projects. Quoting from:
http://pythonhosted.org/six/

"six consists of only one Python file, so it is painless to copy into a
project."

Six seems to be the external dependency that is most likely to cause
problems, and it is a small standalone .py file that is designed for
projects to ship in their source tree.

Shipping has the following advantages:

* Reduces pip dependencies by one;
* Makes mpl less vulnerable to upstream changes in six.  At present, it
  is possible for six to change upstream in pypi and break matplotlib
  installs, requiring a new mpl release;

It does mean that we don't get the lastest six, but I believe this is an
advantage for the reason above.  Now we are so far into the Python 3
era, six should be rather stable.
@tacaswell tacaswell added this to the next point release milestone Jun 7, 2015
@tacaswell

Copy link
Copy Markdown
Member

I am firmly 👎 on this.

six is easy to install (pip install six and conda install six both work) and as you point out six has become very stable so the chances of them producing a breaking change are small.

I also thought we cleaned up the setup.py to automatically install six if needed. I just ran the test of starting with a bare conda enviroment running pip install matplotlib and it installed cleanly (including building numpy from source).

18:25 $ conda list
# packages in environment at /tmp/bare:
#
matplotlib                1.4.3                     <pip>
ncurses                   5.9                           4  
nose                      1.3.7                     <pip>
numpy                     1.9.2                     <pip>
openssl                   1.0.1k                        1  
pip                       7.0.3                    py34_0  
pyparsing                 2.0.3                     <pip>
python                    3.4.3                         1  
python-dateutil           2.4.2                     <pip>
pytz                      2015.4                    <pip>
readline                  6.2.5                         1  
setuptools                17.0                     py34_0  
six                       1.9.0                     <pip>
sqlite                    3.8.4.1                       1  
system                    5.8                           2  
tk                        8.5.18                        0  
xz                        5.0.5                         0  
zlib                      1.2.8                         0  

The things with <pip> in the third column are what pip auto installed.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

Would you mind saying what you think the disadvantage will be?

Is it worth me going back over the issues and mailing list reports of problems with six in order to persuade you? Or are you resolved on this already?

@tacaswell

Copy link
Copy Markdown
Member

What are the install methods for either 1.4.3 or master which result in matplotlib being installed without six either being properly installed or clearly telling the user that they need to install six ? This seems like a sledge hammer solution to a problem that has already been solved.

As a community we should be moving towards many small packages/libraries, bundling easily installable dependencies feels like it is moving the wrong direction.

I just ran the same install experiment with a bare system venv

(venv-bare) ✔ ~ 
19:06 $ pip list
You are using pip version 6.0.8, however version 7.0.3 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
matplotlib (1.4.3)
nose (1.3.7)
numpy (1.9.2)
pip (6.0.8)
pyparsing (2.0.3)
python-dateutil (2.4.2)
pytz (2015.4)
setuptools (12.0.5)
six (1.9.0)

and it worked correctly.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

Projects shipping six or portions of six:

Numpy and sympy went with taking only the bits of six they needed.

I haven't seen any six or python 3 compatibility problems with these projects.

On the other hand, six-related problems seem to be rather common on the matplotlib lists.

I can see the advantage in general of going towards independent packages, but that does have a limit, In the case of six - which is one single .py file - there is - to my eye - no advantage at all for keeping the external dependency - and only disadvantages - of which we have seen fairly common examples.

Can you see another advantage other than the general idea that small independent packages are a good thing?

@matthew-brett

Copy link
Copy Markdown
Contributor Author

I think it's hard to argue the problem has already been solved, on the basis of the fairly frequent emails and issues reporting problems with six - is it worth me collecting those?

@tacaswell

Copy link
Copy Markdown
Member

The issues have to be with 1.4.3 or master and be from install methods we officially support (if some third party package manager is doing a bad job of capturing dependencies that should not be our problem).

Trolling the SO posts at http://stackoverflow.com/search?q=[matplotlib]+No+module+named+six makes me think that the problem is really with our binary windows installers, not with the source.

attn @cgohlke

@cgohlke

cgohlke commented Jun 8, 2015

Copy link
Copy Markdown
Contributor

I can not see anything wrong on Windows.

The bdist_wininst installers don't include or automatically install any dependencies so the user is expected to install them in addition to matplotlib. This is documented at https://github.com/matplotlib/matplotlib/blob/master/INSTALL#L51.

After numpy is installed manually (no wheels are available on pypi), pip install matplotlib installs all required dependencies including six, for example:

C:\Python27>python -m ensurepip
Ignoring indexes: https://pypi.python.org/simple
Collecting setuptools
Collecting pip
Installing collected packages: setuptools, pip
Successfully installed pip-7.0.1 setuptools-16.0

C:\Python27>python -m pip install "numpy-1.9.2+mkl-cp27-none-win_amd64.whl"
Installing collected packages: numpy
Successfully installed numpy-1.9.2

C:\Python27>python -m pip install matplotlib
Collecting matplotlib
  Downloading matplotlib-1.4.3-cp27-none-win_amd64.whl (4.0MB)
    100% |################################| 4.0MB 137kB/s
Collecting pyparsing>=1.5.6 (from matplotlib)
  Downloading pyparsing-2.0.3-py2.py3-none-any.whl
Requirement already satisfied (use --upgrade to upgrade): numpy>=1.6 in C:\Python27\lib\site-packages (from matplotlib)
Collecting pytz (from matplotlib)
  Downloading pytz-2015.4-py2.py3-none-any.whl (475kB)
    100% |################################| 475kB 1.3MB/s
Collecting python-dateutil (from matplotlib)
  Downloading python_dateutil-2.4.2-py2.py3-none-any.whl (188kB)
    100% |################################| 192kB 1.3MB/s
Collecting six>=1.4 (from matplotlib)
  Using cached six-1.9.0-py2.py3-none-any.whl
Installing collected packages: pyparsing, pytz, six, python-dateutil, matplotlib
Successfully installed matplotlib-1.4.3 pyparsing-2.0.3 python-dateutil-2.4.2 pytz-2015.4 six-1.9.0

@cgohlke

cgohlke commented Jun 8, 2015

Copy link
Copy Markdown
Contributor

Btw, python-dateutil requires six. With this PR, parts of matplotlib will still fail if the six package is not installed.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

Christoph - thanks for checking - but I think six gets pulled in with the python-dateutils install, so mpl should not fail (and doesn't on travis):

https://travis-ci.org/matplotlib/matplotlib/jobs/65813427#L703

@matthew-brett

Copy link
Copy Markdown
Contributor Author

I agree - looking back a lot of the problems seem to be people installing from windows exe installers where they will be missing other dependencies.

Thomas - I think you may be making the more general statement that it is not worth fixing problems for non-standard install mechanisms - but it seems reasonable to make failure less likely, if the cost is small, as it is here.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

Independent of the install dependency problem, I think this PR is useful because it makes matplotlib not vulnerable to upstream bug-fixes in six.py.

I still haven't got a full understanding of when people run into trouble with the six dependency. It is possible that there is no practical way of installing matplotlib in a usable form without correctly installing the six package, but I guess I will have to go back over the previous error reports to understand whether that is the case.

@WeatherGod

Copy link
Copy Markdown
Member

The issue that I think has happened is when other projects bring in an
earlier version of six somehow. I don't fully understand it, though because
that should only be local to that particular package. I think we have seen
that in situations like IDLE maybe? There might have also been some issues
reported from Ubuntu users, but I think there are deeper issues on Ubuntu
that are beyond six.

I am +0 on this. In theory, we shouldn't need it, but in practice, it would
make things better for many. My only concern is whether or not we want/need
the updated versions of six. What sort of changes have been going into it
for the past year?

On Mon, Jun 8, 2015 at 3:33 AM, Matthew Brett [email protected]
wrote:

Independent of the install dependency problem, I think this PR is useful
because it makes matplotlib not vulnerable to upstream bug-fixes in six.py.

I still haven't got a full understanding of when people run into trouble
with the six dependency. It is possible that there is no practical way of
installing matplotlib in a usable form without correctly installing the six
package, but I guess I will have to go back over the previous error reports
to understand whether that is the case.

—
Reply to this email directly or view it on GitHub
#4501 (comment)
.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

A short essay. The most obvious problem with six.py is when the user installs from a WIndows exe, in which case presumably none of the dependencies get installed, and missing six is just the marker for the lack of dependencies.

These look as though they are the no-dependencies-with-exe problem:

Obscure pip-ish problems on Windows:

Obscure pyinstaller error on Windows, clashing versions of six:

Macports-specific clash between two installers of six.py:

Obscure six pip problem for PythonAnywhere:

From Gentoo:

On linux - some six version clash:

In summary : most problems are due to exe windows installers, but there are occasional obscure problems with pip installs that are presumably due to the fact that many libraries depend on six, and so matplotlib can be installed into many different environments that already include a copy of six.

I suspect this is much more likely to happen with six than the other matplotlib dependencies like pytz.

So - I still think this PR is worthwhile to:

  • drop the number of obscure errors installing matplotlib;
  • drop the number of obscure errors installing other libraries that depend on six, after installing matplotlib; and
  • make matplotlib not vulnerable to later changes in the six library.

@tacaswell

Copy link
Copy Markdown
Member

A lot of these problems are related to the general issues with python packaging and general dysfunction of windows. I have come to the view that the way to deal with windows is to tell windows users to install one of the all-in-on distributions and push that entire mess off to companies that pay people to deal with it.

At least is the case of c2fff54 I am still convinced that the problem is that other libraries are shipping their own version (or by some other mechanism the user is ending up with multiple versions of six installed and which one is found depends on what gets imported first (and this is probably related to relative vs absolute import?)) and I don't see how installing yet another version of six will help.

I will not merge this and think it is a bad idea, but will not protest any more than I already have if someone else does.

I think a far better solution is to make sure the documentation on how to install is super clear (particularly on windows). Once we start bundling six, we might as well bundle pytz and pyparsing and .... which leads down the road of becoming yet another packaging project which is madness.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

Did I persuade you that there is no way that this version of six can interact with the install for any other package?

I realize that these problems should not happen, if pip was perfect, but it's not very surprising that they do happen sometimes, pip is working in a much more variable environment than all-in-one environments like Anaconda.

I don't think the slippery slope argument applies here. pytz and pyparsing are substantial libraries in their own right, and the authors quite reasonably have no intention that you should ship them. Six on the other hand is specifically a code support file, and is specifically designed to be shipped with your project.

@tacaswell

Copy link
Copy Markdown
Member

I don't know enough about the guts of the import process to be sure that this won't conflict. I believe we will always get the version we think, but not that every other library will always get the version they think. It is clear that this can be done wrong and mpl should be a good citizen in the community.

I also find @cgohlke 's point that even with this, mpl will still be broken is six is not install where python-dateutils can see it, compelling.

This PR needs feed back from other devs.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

Python will treat matplotlib.externals.six as an entirely different module from six - so I can't see any possibility of conflict with other packages

At the moment mpl is not as good a citizen as it can be, through no fault of its own, in that it can run into problems with other installs, and this PR would improve that.

six is designed to be used this way, so the more of us that do that, the fewer problems we are likely to have.

@WeatherGod

Copy link
Copy Markdown
Member

Indeed, the need for six for python-dateutils would severely limit the
effectiveness of this patch. While those errors with Windows users will go
away because six becomes available for matplotlib, they will just be
replaced by errors in python-dateutils' failure to import six (unless they
are also planning to include six in their codebase?).

There was a time that I thought I fully understood the import process, but
then we run into the odd issues every now and then that I just can't seem
to figure out. That is why I like this patch for its practicality and it
shouldn't do any harm to the rest of the python ecosystem. Despite the
python-dateutils issue, I don't really see any harm in merging this. So, it
looks like 1 for and 1 against?

On Fri, Jun 12, 2015 at 3:31 PM, Thomas A Caswell [email protected]
wrote:

I don't know enough about the guts of the import process to be sure that
this won't conflict. I believe we will always get the version we think, but
not that every other library will always get the version they think. It is
clear that this can be done wrong and mpl should be a good citizen in
the community.

I also find @cgohlke https://github.com/cgohlke 's point that even with
this, mpl will still be broken is six is not install where python-dateutils
can see it, compelling.

This PR needs feed back from other devs.

—
Reply to this email directly or view it on GitHub
#4501 (comment)
.

@dopplershift

Copy link
Copy Markdown
Contributor

Given that the project itself mentions the possibility of including the code in the source, I think it's fine to do so. I also don't see huge benefits to getting the latest version of a compatibility layer; either matplotlib works on both Python 2 and 3, or it doesn't.

I haven't paid enough attention to import/windows problems to evaluate benefits, but I don't see any downsides.

@dopplershift dopplershift self-assigned this Jul 11, 2015
@dopplershift

Copy link
Copy Markdown
Contributor

Merged with 8fe495a.

@QuLogic

QuLogic commented Jul 16, 2015

Copy link
Copy Markdown
Member

Packagers will not like bundled libraries, so is there an official method sanctioned for removing six?

@efiring

efiring commented Jul 16, 2015

Copy link
Copy Markdown
Member

No, not that I know of. That would defeat the purpose of bundling six, which we are doing, somewhat reluctantly, as an exception to our usual pattern. I would hope that packagers would respect this and leave it alone. If this is not the case, then I would advocate that we revert this PR. Overall, I lean toward @tacaswell's view of it.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

As I said up in #4501 (comment) - several other big libraries ship six. It's not really a package, it's (deliberately) a single Python file, intended for shipping with other projects. So, the packager would have to be unusually pedantic to object in this case I think.

@WeatherGod

Copy link
Copy Markdown
Member

I think the key distinction here is that what package maintainers hate is when a package installs other packages as part of the install process, possibly causing conflicts with existing installs of that package. What we are doing and simply including the source of six within our codebase. It is accessible only from within our namespace, and it looks like any other part of matplotlib.

@QuLogic

QuLogic commented Jul 18, 2015

Copy link
Copy Markdown
Member

A bundled library is bundled whether or not it conflicts with existing ones. The argument could be made that six is a copylib, but I don't see it listed under the exceptions granted for Fedora (for example).

Of the three examples given above:

  • SciPy - ancient six, which is precisely the reason not to bundle.
  • Pandas - that's a compatibility layer, but it's not six.
  • scikit-learn - the linked file is not six; it's a shim that loads either system six or bundled six (which is intended to be removed by packagers).

The shim in scikit-learn is also used in Astropy (where it originated) and VisPy.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

Thanks for the clarifications. I didn't know there was an explicit thing called a 'copylib', but six clearly qualifies in intent, as per the quote from the six documentation at the top of the PR.

I argued above that upgrading six at the whim of the six packager is a bad idea.

For some background on why this level of package splitting is a bad idea in general:

http://vagabond.github.io/rants/2013/06/21/z_packagers-dont-know-best/
http://vagabond.github.io/rants/2013/06/21/zz_packaging-and-the-tide-of-history/

So I guess the three options are :

  1. revert this, anticipating trouble from strict packagers;
  2. conditional import depending on whether six is installed;
  3. wait and see what packagers say.

I think 2 is a terrible idea, and I have a vague preference for 3 over 1, but I won't be offended either way.

@QuLogic

QuLogic commented Jul 18, 2015

Copy link
Copy Markdown
Member

I am not at all convinced by those rants with regards to six especially given how packagers are able to handle the much more complicated C/C++ dependencies, but I am not in charge here. And I don't package matplotlib (officially), so you wouldn't have to worry about 1 or 3 from me. 😉.

@matthew-brett

Copy link
Copy Markdown
Contributor Author

Sure - those links were probably distracting from the particular case here, I'm sorry for that.

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.

7 participants