Skip to content

Bump minimal pyparsing to 2.0.1 #8754

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

Merged
merged 2 commits into from
Jun 17, 2017
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jun 13, 2017

pyparsing is a pure python package so anyone who can install matplotlib
2.1 can realistically also install a recent-ish pyparsing (in fact pip
will just pick it up; pyparsing 2.0.1 is from 2013). This avoids having to carry around workarounds
for a bunch of old versions.

Also sneak in some version-check related cleanups, e.g. it makes sense to check python version before modules version.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jun 13, 2017
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good, apart from a few changes to uppercase Matplotlib

_python27 = (sys.version_info.major == 2 and sys.version_info.minor >= 7)
_python34 = (sys.version_info.major == 3 and sys.version_info.minor >= 4)
if not (_python27 or _python34):
raise ImportError('matplotlib requires Python 2.7 or 3.4 or later')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'matplotlib' --> 'Matplotlib'

try:
import dateutil
except ImportError:
raise ImportError("matplotlib requires dateutil")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'matplotlib' --> 'Matplotlib'


if not (_python27 or _python34):
raise ImportError('matplotlib requires Python 2.7 or 3.4 or later')
"matplotlib requires pyparsing >= 2.0.1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'matplotlib' --> 'Matplotlib'

@anntzer anntzer force-pushed the pyparsing-version branch from 5971395 to 6ba12f0 Compare June 15, 2017 15:52
@anntzer
Copy link
Contributor Author

anntzer commented Jun 15, 2017

done and made version check warnings consistent.

if not compare_versions(six.__version__, '1.3'):
raise ImportError(
'six 1.3 or later is required; you have %s' % (
six.__version__))
"Matplotlib requires six>=1.3; you have %s" % six.__version__)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't even true; I always have trouble running Matplotlib through Pycharm's profiler because it has six 1.9.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the checked version is 1.10.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, but perhaps we should just drop these checks and assume that pip got the dependencies right...

@anntzer anntzer force-pushed the pyparsing-version branch from 6ba12f0 to f3cc0c6 Compare June 16, 2017 04:21
anntzer added 2 commits June 16, 2017 12:36
pyparsing is a pure python package so anyone who can install matplotlib
2.1 can realistically also install a recent-ish pyparsing (in fact pip
will just pick it up).  This avoids having to carry around workarounds
for a bunch of old broken versions.
It seems logical to check Python version before modules.
@anntzer anntzer force-pushed the pyparsing-version branch from f3cc0c6 to 35ba63e Compare June 16, 2017 19:36
@tacaswell tacaswell merged commit 6ff34d4 into matplotlib:master Jun 17, 2017
@anntzer anntzer deleted the pyparsing-version branch June 17, 2017 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants