-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
parsing: rebuild antlr4 latex parser for 4.10 #23369
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
Conversation
✅ Hi, I am the SymPy bot (v167). I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.11. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
Looks like the tests are failing here. |
If we merge this, we need to make sure the antlr runtime pin is updated in the conda-forge recipe the next time SymPy is released. |
Will take a look at the latex tests, the authors mailmap one looks unrelated. |
Yes, it is: #23363 (comment) |
Puzzled why antlr 4.10 runtime could not parse the version from the serialized parser from the 4.10.1 jar. Took a bit to recognize it was autolev and not latex. I guess antlr target in setup.py should be extended to build the autolev stuff too. |
It doesn't surprise me if there are some inconsistencies there, but they two parsers should definitely both be using the same functionality and tooling where possible. |
PR for building the autolev parser here #23416 |
e197007
to
8435a2d
Compare
A quick update: with the last push latex tests are fine, autolev is still broken because IIRC for some reason the antlr generator visitor is None and so nothing works. It well may be something silly but with all these dynamic imports the root causes tend to be well hidden 😅 |
8435a2d
to
59720b1
Compare
Benchmark results from GitHub Actions Lower numbers are good, higher numbers are bad. A ratio less than 1 Significantly changed benchmark results (PR vs master) Significantly changed benchmark results (master vs previous release) before after ratio
[77f1d79c] [b64cfcdb]
<sympy-1.10.1^0>
+ 99.4±0.2ms 180±0.4ms 1.81 sum.TimeSum.time_doit
Full benchmark results can be found as artifacts in GitHub Actions |
I don't know anything about the antlr parsing so can't review this myself. Is this something that should go into the 1.11 release? If so please add the 1.11 milestone. |
I think it would be good to upgrade this dependency in 1.11. |
Agree. We also need to make sure to update the pin in the conda-forge recipe when the release happens. |
References to other Issues or PRs
Fixes #23355
Fixes #19751
Fixes #22625
Brief description of what is fixed or changed
antlr4 4.10 changed the ATN serialization format (https://github.com/antlr/antlr4/releases/tag/4.10) so that the current latex parser built with 4.7.2 does not work anymore with the latest antlr4-python3-runtime. This PR regenerate a python3 version of the latex grammar using latest antlr 4.10.
Other comments
Wondering about adding an optional dependency on a specific antlr4-python3-runtime version (https://setuptools.pypa.io/en/latest/userguide/dependency_management.html?highlight=install_requires#optional-dependencies)
Release Notes