Linting to fix minor issues #1128
Conversation
lark/utils.py
Outdated
| @@ -4,7 +4,7 @@ | |||
| from collections import deque | |||
|
|
|||
| ###{standalone | |||
| import sys, re | |||
There was a problem hiding this comment.
Did you check that this is truly unused, even when a standalone file is generated?
There was a problem hiding this comment.
I ran the tests and assumed it checked for it. But I just did a test myself, and it is indeed needed. Reverting.
|
Regarding a Cython version: Did you see https://github.com/lark-parser/lark_cython ? |
lark/tree_matcher.py
Outdated
| @@ -88,7 +88,7 @@ class TreeMatcher: | |||
|
|
|||
| def __init__(self, parser): | |||
| # XXX TODO calling compile twice returns different results! | |||
| assert parser.options.maybe_placeholders == False | |||
| assert parser.options.maybe_placeholders is False | |||
There was a problem hiding this comment.
Why change this?
If the user wants to give an object that behaves like False, why should we care?
There was a problem hiding this comment.
I was assuming that this was really looking for the False value here, and not something Falsey. If the later is the case it would be better to replace this with assert not parser.options.maybe_placeholders, otherwise the user has to write the __eq__ method in a very specific way.
For context, the recommendation in pep8 is:
Don’t compare boolean values to True or False using ==
The var == False pattern has a very specific meaning, which is generally not what a user expects. But if that's desired here we can revert it, but based on your comment I think replacing it with not var is the better way to go.
|
Looks good to me. If you can rebase and force-push it, I'll merge it in. (and if you don't know how, I can do it myself) |
|
Should be all set |
|
Thanks! |
I've been looking into the code to try to understand how hard it might be to make a C or Cython variant of this to speed up parsing. As a side effect I've done some minor linting. I've ignored style issues (whitespace etc...) and instead focused on things like unused variables, undefined names, etc...
Issues I've fixed in this PR are:
/home/joncrall/code/lark/lark/parsers/lalr_analysis.py:39:9: F841 local variable 'rules' is assigned to but never used
/home/joncrall/code/lark/lark/reconstruct.py:4:1: F401 'unicodedata' imported but unused
/home/joncrall/code/lark/lark/tree_matcher.py:91:50: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
/home/joncrall/code/lark/lark/parsers/lalr_parser.py:12:1: F811 redefinition of unused 'UnexpectedInput' from line 6
/home/joncrall/code/lark/lark/parsers/lalr_parser.py:12:1: F811 redefinition of unused 'UnexpectedToken' from line 6
/home/joncrall/code/lark/lark/parsers/lalr_analysis.py:288:32: E713 test for membership should be 'not in'
There are a lot of other unused imports but I think those are mostly there to support the standalone functionality, so I didn't touch them.
EDIT: Looks like my editor also clobbered some trailing whitespace.
EDIT2: Also made some changes to the examples:
/home/joncrall/code/lark/examples/advanced/qscintilla_json.py:32:20: F405 'QColor' may be undefined, or defined from star imports: PyQt5.Qt (replaced the import * with explicit imports)
/home/joncrall/code/lark/examples/advanced/_json_parser.py:10:1: F401 'sys' imported but unused