Skip to content

BLD: limit support of pyparsing to <3#21606

Closed
tacaswell wants to merge 1 commit intomatplotlib:v3.5.xfrom
tacaswell:limit_pyparsing35x
Closed

BLD: limit support of pyparsing to <3#21606
tacaswell wants to merge 1 commit intomatplotlib:v3.5.xfrom
tacaswell:limit_pyparsing35x

Conversation

@tacaswell
Copy link
Member

@tacaswell tacaswell commented Nov 12, 2021

PR Summary

Due to a number of issues with the 3.0.x releases of pyparsing having issues,
we are going to for mpl3.5.0 limit to the 2.x series of pyparsing. There is
ongoing work on both the Matplotlib and pyparsing sides and we anticipate that
we will be able to lift this constraint in the next patch release.

Due to one of our dependencies (packaging) also having issues with pyparsing and pinning it back to <3 we have been lulled into a false sense of security about if the v3.5.x branch is passing using pyparsing3 because it was actually testing with pyparsing 2.4.7. Locally I see that on the v3.5.x branch we have:

pyparsing version v3.5.x with Antony Patch
3.0.0 ✅ ✅
3.0.1 ✅ ✅
3.0.2 ❌ ✅
3.0.3 ❌ ✅
3.0.4 ❌ ✅
3.0.5 ❌ ✅

The tests that fail are all of a dot moving from above a letter to in front of it. If I cherry-pick the commit from #21501 (ec497ae) back to v3.5.x the tests pass. However, given the discussion with @anntzer and @ptmcg I'm not sure if that that fix is "right" or not. From #21501 (comment)

Ok. There is a bug in pyparsing, but fixing it doesn't completely solve your problem.

I do not expect a future version of pyparsing to fix it for us so we will need some additional changes on our side

For mpl3.5.0 we need to merge either this PR or #21501. On one hand, merging this PR is "safe" in that we are pretty confident that it won't break our users and buys more time to figure out what the right fix on our side is and in 3.5.1 we can exclude any version of pyparsing we do not want to / can not support (due to known bugs or performance versions). On the other hand, if we merge #21501 we do not have to do any fancy pinning on pyparsing and if the solution there is not ideal we can fix it up in a patch release.

On balance, I think that merging #21501 and closing this PR is the right path forward.

Details 
===================================================================================================================================== short test summary info ======================================================================================================================================
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[png-mathtext-cm-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 2.308):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[png-mathtext-stix-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 16.782):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[png-mathtext-stixsans-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 17.033):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[png-mathtext-dejavusans-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 20.620):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[png-mathtext-dejavuserif-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 20.436):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[pdf-mathtext-cm-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 5.354):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[pdf-mathtext-stix-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 27.009):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[pdf-mathtext-stixsans-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 26.110):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavusans-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 31.526):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavuserif-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 32.275):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[svg-mathtext-cm-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 2.238):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[svg-mathtext-stix-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 16.225):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[svg-mathtext-stixsans-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 17.217):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[svg-mathtext-dejavusans-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 20.396):
FAILED lib/matplotlib/tests/test_mathtext.py::test_mathtext_rendering[svg-mathtext-dejavuserif-0] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 20.167):

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

@tacaswell
BLD: limit support of pyparsing to <3
2eb7965 
Due to a number of issues with the 3.0.x releases of pyparsing having issues,
we are going to for mpl3.5.0 limit to the 2.x series of pyparsing.  There is
ongoing work on both the Matplotlib and pyparsing sides and we anticipate that
we will be able to lift this constraint in the next patch release.
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Nov 12, 2021
@tacaswell tacaswell added this to the v3.5.0 milestone Nov 12, 2021
@anntzer anntzer mentioned this pull request Nov 14, 2021
Closed
@anntzer
Copy link
Contributor

anntzer commented Nov 14, 2021

My understanding is that #21501 should always work, even if it is not the fastest implementation. I have a fairly large refactor in the works (#21448 and followup patches) which together yield a significant speedup (timings included in comments), so I wouldn't worry too much about the perf of the #21501 patch as it's just a temporary stopgap for 3.5.
Also pinging @ptmcg here as he's certainly the most knowledgeable on the topic :-)

@anntzer anntzer mentioned this pull request Nov 14, 2021
Merged
7 tasks
@tacaswell tacaswell closed this Nov 14, 2021
@tacaswell tacaswell deleted the limit_pyparsing35x branch November 14, 2021 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Projects

None yet

Milestone

v3.5.0

Development

Successfully merging this pull request may close these issues.

2 participants

@tacaswell @anntzer