-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Remove Python2 target. #4331
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
Remove Python2 target. #4331
Conversation
Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
… page when pointing to the directory. Naming the document as "index.md" is not a standard practice. Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
doc/target-agnostic-grammars.md
Outdated
| in order to add context-sensitive parsing to what would normally be a context-free grammar. | ||
|
|
||
| For example: | ||
| * In Fortran90, [lines that being with a 'C' in column 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplier is better. Actions are usually in a grammar to spport predicates. No need to single out actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'being' vs 'begin' is, I believe, a typo
|
I believe we're missing an announcement in the top-level read-me ? |
|
(Changing to "Draft" to resolve issues.) |
In /README.md? There is a mention of the targets that are currently available here, a list of author/contributor with target name and specifically for Python here, and a link on the text "ANTLR code generation targets" here. Note, the title used in the link is not the same as the title of the document itself, which is "Runtime Libraries and Code Generation Targets". Standard industry and academic documentation should have the titles agree, letter for letter! |
|
Yes, in /README.md |
|
Signed-off-by: Ken Domino <[email protected]>
|
Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
|
…tside this repo that depend on the existence of index.md. The files are renamed back, including content but for links only. Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
…seful Information" section of Python2. Fix count of number of targets. Enumerate targets nearby the count number so that it may be updated more easily in the future. Signed-off-by: Ken Domino <[email protected]>
… grammar, style, spelling in target-agnostic-grammars.md. Signed-off-by: Ken Domino <[email protected]>
|
@parrt blessed |
Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
Signed-off-by: Ken Domino <[email protected]>
|
Wordsmithed the heck out of the two .md files. Done. |
|
Changed next release to 4.14.0, not 4.13.1. Thanks for the hard work cleaning this up @kaby76 ! |
|
This leaves people using Jython with the last available version being partially broken (see PR #4313). While CPython 2.7 is effectively dead, Jython is still quite alive (and currently stuck at 2.7, sadly). Would it be possible to still have 4.13.1 as the last available version for Jython users (along with a Python wheel package for the Python2 runtime this time) and then get rid of the Python2 target? |
|
How do you test a 4.13.1 release? Or just release it untested? |
|
If needed, I can take a look at how much work it would take to run the test suite under Jython. I haven't looked at it yet, but I guess that besides an added maven dependency for https://mvnrepository.com/artifact/org.python/jython-standalone/2.7.3, script-wise it should be a matter of replacing |
|
@agatti how about forking 4.13.0, apply the missing patch and publish an antlr4-jython-runtime ? Then you can gradually evolve it to follow python upgrades ? |
|
@ericvergnaud This can work just for that release. Generated Python3 code has type annotations and it cannot be used as such. If it is acceptable to modify the Python generator to use comment-based typing instead of the usual inline typing declarations then this may be an option. |
|
@agatti sorry but we won't be maintaining a python 2 runtime going forward. People in need of antlr4 parsers generated for Python 2 will need to stick to antlr4 4.13.0 anyway. |
|
@ericvergnaud I may have expressed myself the wrong way, apologies for that. My point was that if the Python3 generator can no longer output code like: # This class defines a complete listener for a parse tree produced by CParser.
class CListener(ParseTreeListener):
# Enter a parse tree produced by CParser#primaryExpression.
def enterPrimaryExpression(self, ctx:CParser.PrimaryExpressionContext):
pass
# ...and would output code like this instead: # This class defines a complete listener for a parse tree produced by CParser.
class CListener(ParseTreeListener):
# Enter a parse tree produced by CParser#primaryExpression.
def enterPrimaryExpression(self, ctx):
# type: (CParser.PrimaryExpressionContext) -> None
pass
# ...it would still be PEP-0484 compliant and somebody else (I guess that would be me) would fork the runtime for Jython and bring it forward when needed. No Python2 runtime would have to be kept alive on your end. |
|
Ok, there may be a couple more |
|
Your proposed approach would prevent the Python 3 generator from using any Python 3 feature not available in Python 2. |
|
@ericvergnaud Fair enough. I'll weigh my options and see what to do then. Thank you for your time. |
This PR fixes #4325.
This change removes the Python2 target from Antlr.
Github Actions removed support of testing of Python 2.x on June 20, 2023. Consequently, every PR (#4330, #4329, #4323, #4321, #4319, ...) fails because the workflow cannot install Python 2.x. In addition, official support from python.org for 2.x stopped several years ago. Keeping code around that we cannot develop or test is illogical. Old versions of Antlr4 exist for people to use, but going forward, it has to stop.
The instructions for the Python target have been updated. (NB: although the instructions specifically say to "not mix code and documentation updates in the sample pull request", it does not make sense to do that for this major PR.) References to Python2 are removed. I also took the liberty to update the content of the instructions itself, which were not very detailed. Minimum Python requirements were added. A functioning code example for a minimal parser driver was added. Functioning examples for Antlr tree traversal using a visitor and a listener, which evaluate an expression of an expression grammar, were added.
The instructions for the Python target had contained a section on target-agnostic grammars. This information is unrelated to Python target itself, but too important to delete. I moved the content to a new file, and rewrote the text in a more formal style.