BLD: limit support of pyparsing to <3 #21606
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
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)
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.
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).