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

Pyparsing #2279

Merged
merged 5 commits into from Aug 12, 2013
Merged

Pyparsing #2279

merged 5 commits into from Aug 12, 2013

Conversation

jenshnielsen
Copy link
Member

Bump minimum pyparsing version to 2.0.1. By doing this we can replace the deprecated << with <<= and simplify the version logic for pyparsing. In addition a no longer relevant work around for pyparsing <2.0.0 on python 3 has been removed. This fixes #2240 and #2244

…<= and remove no longer relevant work around.
@mdboom
Copy link
Member

mdboom commented Aug 7, 2013

I appreciate the time you've taken to do this. Unfortunately, I don't think we can merge this until 2.0.1 is available as a package in the major Linux distros, otherwise we break things there.

@jenshnielsen
Copy link
Member Author

Yes that is an issue. I mostly did it to get rid of all the warnings in my own install. Note that mpl already requires 2.0.0 on python3 while Ubuntu 13.04 only ships 1.5.7 so the issue already exists. Se also #1788.

@mdboom
Copy link
Member

mdboom commented Aug 7, 2013

Yeah -- unfortunately I think Python 3 users are used to living on the bleeding edge and can update their pyparsing with pip. (And IIRC, pyparsing < 2.0 on Python 3 has lots of bugs and simply doesn't work with matplotlib). For everyone else, I think we want to try to work with the commonly installed packages. It is annoying though.

An alternative approach might be to define a local function, call it assign (or something) like this:

def assign(ref, val):
    if hasattr(ref, '__ilshift__'):
        ref.__ilshift__(val)
    elif hasattr(ref, '__lshift__'):
        ref.__lshift__(val)

and then replace instances of x <<= y with assign(x, y).

An alternative that might would would be to monkey patch pyparsing as such:

if not hasattr(Forward, '__ilshift__'):
    Forward.__ilshift__ = Forward.__lshift__

…oth Python 2 and 3 without loads of deprecation warnings.
@mdboom
Copy link
Member

mdboom commented Aug 7, 2013

@jenshnielsen: I've filed a pull request to your branch that uses the monkey-patching approach. We now have support for pyparsing >= 1.5.6 (except for 2.0.0 with Python 2.x) without spewing deprecation warnings anywhere.

@jenshnielsen
Copy link
Member Author

The pull request is at jenshnielsen#1 Unfortunatelyit fails on python 3 because
"<<=" is essentially broken in version 2.0.0 of pyparsing which is what travis installs.

See http://pyparsing.wikispaces.com/News

Pyparsing 2.0.1 also fixes a bug in the implementation of the '<<=' operator, so that it can be used in place of '<<' within problems with recursive grammars.

If we do this we should probably exclude 2.0.0 in all cases since it is complaining about a depreciated behaviour and recommends a broken one. But perhaps we should just filter the warning for now

Travis also times out with the python 2 tests in the pip install but that is most likely just a travis glitch.

@mdboom
Copy link
Member

mdboom commented Aug 7, 2013

Ok -- I've updated jenshnielsen#1. It excludes the use of 2.0.0 in all cases (I don't have a problem with this, as it was a very short-lived, transient release).

I then also simplified the pyparsing installation on travis to download the latest version (currently 2.0.1) in all cases.

@jenshnielsen
Copy link
Member Author

I merged the pull request. I think this looks sensible. The only thing that has changed is that 1.5.6 and 1.5.7 are now valid versions with python3.x previously 2.0.0 was required. Was there a particular reason for this?

@mdboom
Copy link
Member

mdboom commented Aug 7, 2013

I think it just ended up that way accidentally through a circuitous route. The block of code to turn off packrat parsing for pyparsing 1.x on python 3.x was added after the version check for pyparsing 2.x and above, and the latter never got updated. I think now we have sort of "maximum possible" coverage of pyparsing versions, and minimal warnings displayed. Thanks for pushing for this improvement.

@mdboom
Copy link
Member

mdboom commented Aug 8, 2013

@jenshnielsen: Do you feel this is ready to merge?

@jenshnielsen
Copy link
Member Author

I think it looks good. Perhaps someone should test it manually with pyparsing 1.5.6. I will see if I can get around to do it over the weekend.

@mdboom
Copy link
Member

mdboom commented Aug 8, 2013

I tested with pyparsing 1.5.6, 1.5.7 and 2.0.1 under Python 2.x and 3.x on Linux (though I doubt the platform matters in this case).

@jenshnielsen
Copy link
Member Author

Great then I think it is ready to go.

mdboom added a commit that referenced this pull request Aug 12, 2013
@mdboom mdboom merged commit 04b9ab9 into matplotlib:v1.3.x Aug 12, 2013
@jenshnielsen jenshnielsen deleted the pyparsing branch September 10, 2013 14:09
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.

None yet

2 participants