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

Lower minimum pyparsing version to 1.5.2 #1788

Closed
mgiuca-google opened this issue Feb 27, 2013 · 11 comments
Closed

Lower minimum pyparsing version to 1.5.2 #1788

mgiuca-google opened this issue Feb 27, 2013 · 11 comments

Comments

@mgiuca-google
Copy link
Contributor

As of 0c6f12a (26 Feb 2013), Matplotlib on Python 2.0 requires pyparsing 1.5.6 or higher. Unfortunately, Ubuntu 12.04 LTS (the most recent long-term support release of Ubuntu) only has pyparsing 1.5.2, so it would be desirable to lower the minimum required version in order to be able to run the latest version of Matplotlib on Ubuntu LTS without having to manually install other libraries.

I have edited the pyparsing version requirement in __init__.py from 1.5.6 to 1.5.2 and run the test suite against pyparsing 1.5.2 included with Ubuntu 12.04. All of the tests still pass. Was there a good reason to set it to 1.5.6, or was it just the version you happened to have handy to test with? Unless there is a good reason, I recommend lowering the minimum version to 1.5.2.

@mgiuca-google
Copy link
Contributor Author

Note: here is the list of pyparsing versions on different Ubuntu releases. Pyparsing 1.5.6 is included from 12.10 onwards, so this is just a problem for LTS users.

@mdboom
Copy link
Member

mdboom commented Feb 27, 2013

The impetus for this was because:

a) on Python 3, pyparsing 2.0.0 is required (for a critical bugfix)
b) pyparsing 2.0.0 has a number of deprecated APIs that we're still using, so it spews warnings everywhere
c) the replacements for those deprecated APIs weren't available until pyparsing 1.5.6
d) therefore, we have to use at least pyparsing 1.5.6 everywhere in order to not have a bunch of if statements everywhere.

While I understand that Ubuntu prior to 12.10 may not have pyparsing 1.5.6, isn't this only a minor problem for people who are installing matplotlib from source anyway? The new setup infrastructure should upgrade pyparsing automatically using pip if they are installing matplotlib with the --upgrade option anyway. When it comes time to upgrade the matplotlib Ubuntu package, upgrading pyparsing as well can be done then.

@mdboom mdboom mentioned this issue Feb 27, 2013
@mgiuca-google
Copy link
Contributor Author

OK, if you had a good reason to require 1.5.6, then that's fine. I was just checking in case it was an arbitrary decision and it was possible to lower the requirements.

(Should there be a test which fails when using pyparsing 1.5.2? Currently all tests pass.)

@mdboom
Copy link
Member

mdboom commented Feb 28, 2013

master has a version check for pyparsing upon importing matplotlib, so I think that causes all tests to fail as a result (but that was put in after the version bump, so you may not have been testing with that).

I'm going to go ahead and close this, but feel free to reopen if there's a better way to deal with this. Thanks.

@mdboom mdboom closed this as completed Feb 28, 2013
@mgiuca-google
Copy link
Contributor Author

Yeah with the version check in place, the tests fail. I'm talking about if I comment out the version check, then all the tests pass. So while the tests test that the version check works correctly, they don't test whatever it is that is actually requiring pyparsing 1.5.6 ("the replacements for those deprecated APIs weren't available until pyparsing 1.5.6"). So the test suite does not prove that 1.5.6 is actually required.

@mdboom
Copy link
Member

mdboom commented Feb 28, 2013

I haven't made the API updates yet -- I first wanted to ensure that Travis would handle the version upgrade etc...

@mgiuca-google
Copy link
Contributor Author

Ah, I understand.

@juliantaylor
Copy link
Contributor

@mdboom can you point me to these this critical bugfix for python3 pyparsing and how it affects matplotlib?
the pyparsing changelog does not mention any bugfix at all and the source diff is useless because of the source conversion.

the hard requirement on 2.0.0 for python3 is very unfortunate, as only the most bleeding edge distributinos can even package it, all others must stick with the 1.5 version to continue to support python 2.6

@mdboom
Copy link
Member

mdboom commented Apr 18, 2013

I reported the problem we were having with pyparsing and matplotlib here, but I wasn't the first to report it:

http://sourceforge.net/mailarchive/message.php?msg_id=30107833

The pyparsing CHANGELOG entry is:

- Fixed a bug in the Py3 version of pyparsing, during exception
  handling, reported by Catherine Devlin - thanks Catherine!

It sounds less severe than it is. Every backtrack in a pyparsing parser involves exception handling, so this basically made it useless on Python 3.

Note that in matplotlib, we require 2.0.0 only for Python 3. On Python 2, the requirement is 1.5.6. My understanding it this (from private comm with Paul McGuire):

I had packaged up 1.5.7 much like 1.5.6 was, with both Python 2.x and 3.x
compat versions bundled into one. But I think I'll put out 1.5.7 as Python 2.x
only, and a 1.6 as Python 3.x only, and try to work only on the 1.6.x 
releases from now on.

So, is it not possible for the distros to ship a different version of pyparsing for each version of python?

@juliantaylor
Copy link
Contributor

its certainly possible to package it, but only the newest ones will have it. E.g. debian based distros are just now about to go into their next release cycle.

Is it possible to change the version check into a feature/bug test?
That would allow distributions to backport the fix and users can still use unmodified git HEAD matplotlib with all dependencies coming from packages.
Btw the matplotlib 1.2.1 python testsuite succeeds with 1.5.6, is this missing coverage fixed in HEAD?

@mdboom
Copy link
Member

mdboom commented Apr 19, 2013

I've created a PR #1927. Let's continue the discussion there.

@jenshnielsen jenshnielsen mentioned this issue Aug 7, 2013
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

No branches or pull requests

3 participants