Py runtime: Move to relative imports #4705
Conversation
pelson
force-pushed
the
feature/pyruntime-imports
branch
from
3bd8e91
to
8d82b49
Compare
| @@ -52,7 +52,7 @@ def removeErrorListeners(self): | |||
| def getTokenTypeMap(self): | |||
| tokenNames = self.getTokenNames() | |||
| if tokenNames is None: | |||
| from antlr4.error.Errors import UnsupportedOperationException | |||
| from .error.Errors import UnsupportedOperationException | |||
There was a problem hiding this comment.
I'm guessing that the reason these imports were made lazy was because of circular import issues... This MR should start to straighten that out.
| from antlr4.error.ErrorStrategy import BailErrorStrategy | ||
| from antlr4.error.DiagnosticErrorListener import DiagnosticErrorListener | ||
| from antlr4.Utils import str_list | ||
| from .Token import Token |
There was a problem hiding this comment.
In the future I would recommend making these explicit export based imports, as mentioned in https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-no-implicit-reexport.
I didn't do that here because I would try to add some validation of this expectation.
|
@pelson thanks for this. LGTM apart from my 2 comments. |
|
Thanks @ericvergnaud - I think you forgot to hit submit review. Comments missing. |
| import importlib | ||
| from antlr4 import * | ||
|
|
||
| from . import * |
There was a problem hiding this comment.
This is the only place where import * is not fixed - I would be happy to follow-up in another PR to make the names being used explicit.
| from antlr4.error.Errors import LexerNoViableAltException | ||
| from antlr4.tree.Tree import ParseTree | ||
| from antlr4.tree.Trees import Trees | ||
| from ..Lexer import Lexer |
There was a problem hiding this comment.
This file is generated, and generated files cannot use relative imports since they normally sit outside the antlr code base. Please revert.
There was a problem hiding this comment.
Is XPath.py really auto-generated? I don't see it in https://github.com/antlr/antlr4/blob/dev/doc/releasing-antlr.md#build-xpath-parsers (unlike XPathLexer.py which definitely is auto-generated)
| @@ -1,5 +1,11 @@ | |||
| # Generated from XPathLexer.g4 by ANTLR 4.13.1 | |||
| from antlr4 import * | |||
| from ..Lexer import Lexer | |||
There was a problem hiding this comment.
This file is generated, and generated files cannot use relative imports since they normally sit outside the antlr code base. Please revert
Signed-off-by: Phil Elson <[email protected]>
pelson
force-pushed
the
feature/pyruntime-imports
branch
from
8d82b49
to
06417ca
Compare
This also fixes a few
from antlr4 import *, which is considered bad practice unless being done to expose an interface (https://peps.python.org/pep-0008/#imports)My changes seem to modify generated files in
antlr4.xpath. I didn't yet look at fixing the generator.There remains some significant issues withe the namespaces being exposed in the runtime - I would be happy to follow up after this change to tighten the names being made publicly available.