lark
lark copied to clipboard
style: pre-commit basic config
This adds a basic pre-commit-config, a tox job running it (to help new contributors - usually you should run pre-commit directly to take advantage of it's start-up speed), and a GitHub job. Pre-commit is from Anthony Sottile, the developer behind flake8, pytest, tox, and other tools. You can read more about it at https://pre-commit.com or https://scikit-hep.org/developer/style. You can see it on many of the PyPA repos, like build, pip, pipx, cibuildwheel, etc. If you want, there's also a pre-commit.ci service that will auto-update your pre-commit file and auto-fix PRs, and is ultra-fast.
If you like this, I can add a few more checks, starting with the basics. MyPy can be moved to pre-commit as well. I won't add black or isort unless you ask for them, but I think you'll really like pyupgrade.
By the way, you can exclude files, even per-check, so let me know if there's any that intentionally unusual (like wrong file endings) for testing purposes. I can also ignore SVGs, etc, quite easily.
- chore: add pre-commit basic checks
- style: run pre-commit - basic checks
FYI, I mostly run pre-commit run -a like you might run nox or tox, but you can also run pre-commit install and have it run in "precommit hook mode", where it checks on commit unless you pass -n.
Why did it modify the SVGs?
MyPy can be moved to pre-commit as well
It sounds like a good fit for pre-commit. But moved from where?
pyupgrade looks nice! (with --keep-percent-format, because it doesn't support f-strings, it seems). Though I'm not a fan of pre-commits that change files beyond whitespace. But as long as it's a manual run, it's a nice addition!
Is there a way to split the pre-commit, so that checks run automatically (like with install), but modifications need to be run manually?
~Another question - if the files get modified by the CI, who is signed on the commit?~ Disregard. It's better that the CI won't make any changes :)
Why did it modify the SVGs?
Probably for consistent end lines or extra whitespace.
But moved from where?
Currently it's sitting in tox.ini.
because it doesn't support f-strings, it seems
What doesn't support f-strings? Pyupgrade will convert % to f-strings if there's no possible output change and the expression in the f-string is very simple. I know of some other tools that are a bit more aggressive that can be used to convert the rest. f-strings are faster than % or .format[^1].
Though I'm not a fan of pre-commits that change files beyond whitespace
There's not that much you can do beyond whitespace and a few minor things in Python. We don't have an equivalent to Rubocop for Python. Most tools are just linters only.
But as long as it's a manual run, it's a nice addition
Pre-commit is just the name of the tool. It's not a very good name.
Is there a way to split the pre-commit, so that checks run automatically (like with install), but modifications need to be run manually?
You can set stages on all hooks. There is a manual hook stage, but you have to specify it when running pre-commit. I'm not sure if you can specify a "non-precommit" state that runs manually without adding --hook-stage manual. I'd generally not bother, and just don't use the precommit mode if you don't want it running. (As I mentioned, I rather rarely install and use it as a git precommit hook).
It's worth trying out, though, if you haven't. It's very well done - it handles partial checkings, for example, by copying to a temp directory and running there, etc. I'd recommend trying out everything on the page I shared, one at a time, eventually. I'll only be adding this (I was having issues with line endings on my last PR), pyupgrade, and maybe some linting checks - I'm really after improving the typing coverage a bit so I can measure the impact of mypyc. MyPy and Black both use mypypc to get nice speedups. Style formatting (Black, isort, etc) are a personal choice, and not a usually a good contribution unless asked for.
It's better that the CI won't make any changes
You can still activate pre-commit.ci, but leave changes off. We do that in pypa/build. You still get the fast checks & weekly updates.
I'll add a couple of exclusions, and repush, and we can go from there. Probably in an hour or two.
[^1]: Python 3.11 allows very simple % formatting to be as fast as f-strings.
Probably best to exclude SVGs.
Currently it's sitting in tox.ini.
Doesn't it make sense to have it in both?
Pyupgrade will convert % to f-strings
That would be great. When I tried, it converted them to .format() calls instead.
Style formatting (Black, isort, etc) are a personal choice
Good call. I'm not a fan of reformatters, and have bad experience with isort.
You can still activate pre-commit.ci, but leave changes off
Sounds good.
I'm also curious to know if the type info produces any speed-up, and by how much.
Probably best to exclude SVGs.
I'll also exclude .lark files in tests.
Doesn't it make sense to have it in both?
Sure, but it should be defined once (in pre-commit), and simply run from there via tox.
When I tried, it converted them to .format() calls instead
Ah, you forgot to pass --py36-plus. It tries to upgrade to 2.7 syntax without any target.
bad experience with isort.
Probably some time ago? It was reworked and it very good these days. And Black is great - once you get used to reading blacked code, it becomes really easy to read lots of code bases.
Here's the report so you can see all changed files and why they were changed:
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for merge conflicts................................................Passed
check for broken symlinks............................(no files to check)Skipped
check toml...............................................................Passed
check yaml...............................................................Passed
debug statements (python)................................................Passed
fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook
Fixing LICENSE
Fixing lark/parsers/lalr_interactive_parser.py
Fixing lark/parsers/lalr_parser.py
Fixing docs/requirements.txt
Fixing setup.py
Fixing examples/grammars/README.md
Fixing docs/visitors.rst
Fixing examples/standalone/README.md
Fixing docs/features.md
Fixing examples/advanced/py3to2.py
Fixing examples/advanced/error_handling.py
Fixing lark/parsers/earley_common.py
Fixing docs/ide/app/app.py
Fixing docs/ide/app/html5.py
Fixing docs/conf.py
Fixing examples/advanced/python2.lark
Fixing CHANGELOG.md
Fixing examples/advanced/error_reporting_earley.py
Fixing examples/composition/storage.lark
Fixing docs/ide/app/files.json
Fixing docs/_static/sppf/sppf.html
Fixing docs/philosophy.md
Fixing examples/advanced/error_reporting_lalr.py
Fixing test-requirements.txt
Fixing docs/tools.md
Fixing README.md
Fixing docs/recipes.md
Fixing docs/Makefile
Fixing examples/indented_tree.py
Fixing examples/composition/README.md
Fixing tests/test_lexer.py
Fixing .github/ISSUE_TEMPLATE/other.md
Fixing examples/composition/main.py
Fixing .gitignore
Fixing docs/ide/app/examples.py
Fixing tests/test_grammar.py
Fixing examples/advanced/templates.py
Fixing examples/standalone/json_parser_main.py
Fixing examples/tests/no_newline_at_end.lark
Fixing examples/advanced/reconstruct_python.py
Fixing lark/tools/standalone.py
Fixing lark/__pyinstaller/__init__.py
Fixing docs/classes.rst
Fixing examples/advanced/python_parser.py
mixed line ending........................................................Failed
- hook id: mixed-line-ending
- exit code: 1
- files were modified by this hook
examples/composition/README.md: fixed mixed line endings
examples/advanced/py3to2.py: fixed mixed line endings
CHANGELOG.md: fixed mixed line endings
examples/grammars/README.md: fixed mixed line endings
fix requirements.txt.....................................................Failed
- hook id: requirements-txt-fixer
- exit code: 1
- files were modified by this hook
Sorting docs/requirements.txt
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook
Fixing lark/parsers/lalr_interactive_parser.py
Fixing examples/advanced/py3to2.py
Fixing tests/test_python_grammar.py
Fixing lark/lark.py
Fixing lark/exceptions.py
Fixing lark/parsers/earley_common.py
Fixing examples/grammars/verilog.lark
Fixing lark/ast_utils.py
Fixing README.md
Fixing lark/parsers/earley.py
Fixing lark/parser_frontends.py
Fixing lark/visitors.py
Fixing docs/classes.rst
Fixing lark/grammars/python.lark
Fixing docs/how_to_develop.md
Fixing tests/test_cache.py
Fixing lark/tools/__init__.py
Fixing tests/test_parser.py
Also, if you rebase-and-merge or classic merge, then you can add the second commit's sha to your .git-ignore-revs file and GitHub will ignore that commit in history (blame) view (recent addition to GitHub).
Only thing I noticed was that the comment in docs/requirements.txt is now misplaced due to sorting it.
Thanks you for your contribution!
Sorry that it took so long to review. My mind's been on other projects.
No problem, my mind has been too. I'll pop back here once and a while and try to slowly work on more typing, still curios to try mypyc.