modernization of proselint #1361
+8,283
−7,197
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,19 +11,19 @@ jobs: | |
| shell: bash | ||
| strategy: | ||
| matrix: | ||
| python: ["3.10"] | ||
| steps: | ||
| - name: "[INIT] Checkout repository" | ||
| uses: actions/checkout@v4 | ||
| - name: "[INIT] Install Python ${{ matrix.python }}" | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ${{ matrix.python }} | ||
| - name: "[INIT] Install Poetry" | ||
| uses: snok/[email protected] | ||
| - name: "[INIT] Restore dependency cache" | ||
| id: cache-restore | ||
| uses: actions/cache@v3 | ||
| with: | ||
| path: | | ||
| ~/.cache/pip | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,9 +13,9 @@ jobs: | |
| python: ["3.10"] | ||
| steps: | ||
| - name: "[INIT] Checkout repository" | ||
| uses: actions/checkout@v4 | ||
| - name: "[INIT] Install Python ${{ matrix.python }}" | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ${{ matrix.python }} | ||
| - name: "[INIT] Install Poetry" | ||
|
|
@@ -30,9 +30,12 @@ jobs: | |
| key: ${{ runner.os }}-py${{ matrix.python }}-${{ hashFiles('**/poetry.lock') }} | ||
| - name: "[INIT] Install dependencies" | ||
| if: ${{ !steps.cache-restore.outputs.cache-hit }} | ||
| run: poetry install | ||
| # - name: "[EXEC] Lint" | ||
| # run: ./utils ci lint | ||
| # - name: Run Pre-Commit Tests 🧪 | ||
| # uses: pre-commit/[email protected] | ||
|
|
||
| test-cover: | ||
| name: Test & Cover | ||
| if: "!(contains(github.event.head_commit.message, '[skip_ci]'))" | ||
|
|
@@ -42,13 +45,14 @@ jobs: | |
| shell: bash | ||
| strategy: | ||
| matrix: | ||
| # 5 versions / years of coverage | ||
| python: ["3.8", "3.9", "3.10", "3.11", "3.12", "pypy3.9", "pypy3.10"] | ||
| os: [ubuntu-latest, macos-latest, windows-latest] | ||
| steps: | ||
| - name: "[INIT] Checkout repository" | ||
| uses: actions/checkout@v4 | ||
| - name: "[INIT] Install Python ${{ matrix.python }}" | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ${{ matrix.python }} | ||
| - name: "[INIT] Install Poetry" | ||
|
|
@@ -64,10 +68,14 @@ jobs: | |
| - name: "[INIT] Install dependencies" | ||
| if: ${{ !steps.cache-restore.outputs.cache-hit }} | ||
| run: pip install poetry && poetry install | ||
| - name: "[EXEC] Test build package" | ||
| run: poetry build | ||
| - name: "[EXEC] Unit test" | ||
| run: poetry run pytest | ||
| # - name: "[EXEC] Test" | ||
| # run: ./utils ci test --coverage | ||
| # - name: "[EXEC] Upload coverage to Codecov" | ||
| # uses: codecov/[email protected] # TODO: maybe not on every py-ver? | ||
| # with: | ||
| # fail_ci_if_error: true | ||
| # flags: ${{ matrix.os }},py${{ matrix.python }} | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||
|
|
||||||
| line-length = 80 | ||||||
| target-version = "py38" | ||||||
|
|
||||||
| [lint] | ||||||
| select = [ | ||||||
| "A", # flake8-builtins | ||||||
| # "ANN", # flake8-annotations TODO activate | ||||||
| # "ARG", # flake8-unused-arguments | ||||||
| "B", # Bugbear | ||||||
| "C", | ||||||
| "COM", # flake8-commas | ||||||
| # "CPY", # flake8-copyright | ||||||
| "C4", # flake8-comprehensions | ||||||
| "DTZ", # flake8-datetimez | ||||||
| # "D", # pydocstyle, TODO: activate | ||||||
| "E", # pycodestyle errors | ||||||
| # "ERA", # eradicate commented out code | ||||||
| "F", # pyflakes | ||||||
| "FA", # flake8-future-annotations | ||||||
| # "FBT", # boolean traps | ||||||
| "FLY", # flynt | ||||||
| "FURB", # refurb | ||||||
| "G", # flake8-logging-format | ||||||
| "I", # incomplete isort | ||||||
| "INP", # flake8-no-pep420 | ||||||
| "LOG", # flake8-logging | ||||||
| # "N", # naming | ||||||
| "NPY", # NumPy-specific rules | ||||||
| "PD", # pandas-vet | ||||||
| "PERF", # Perflint | ||||||
| "PL", # Pylint | ||||||
| "PTH", # flake8-use-pathlib | ||||||
| "PYI", # flake8-pyi | ||||||
| "RET", # flake8-return | ||||||
| "RUF", # Ruff-specific rules | ||||||
| "S", # bandit, security | ||||||
| "SLF", # flake8-self | ||||||
| "SIM", # flake8-simplify | ||||||
| "TID", # flake8-tidy-imports | ||||||
| "TCH", # flake8-type-checking | ||||||
| "T10", # flake8-print | ||||||
| "UP", # pyupgrade | ||||||
| "W", # pycodestyle warnings | ||||||
| "YTT", # flake8-2020 | ||||||
| ] | ||||||
| ignore = [ | ||||||
| "PLR2004", # magic values | ||||||
| "COM812", # trailing comma -> done by formatter | ||||||
| # TODO: below is temporary | ||||||
|
|
||||||
| ] | ||||||
| preview = true | ||||||
|
|
||||||
| [lint.per-file-ignores] | ||||||
| "tests/**" = ["S", "N8", "SLF001", "PLR2004"] | ||||||
| "proselint/checks/**" = ["RUF001", "RUF002", "RUF003", "TID252"] | ||||||
| "scripts/**" = ["E501"] | ||||||
|
|
||||||
| "plugins/**" = ["INP001", "RUF012"] | ||||||
| "scripts_web/**" = ["S1", "SIM", "RET", "TCH"] | ||||||
|
|
||||||
| [lint.mccabe] | ||||||
| # Unlike Flake8, default to a complexity level of 10. | ||||||
| max-complexity = 11 | ||||||
|
|
||||||
| [lint.isort] | ||||||
| force-single-line = true | ||||||
|
Comment on lines
+67
to
+68
There was a problem hiding this comment.
Suggested change
I think this should be removed. It makes imports unwieldy. |
||||||
|
|
||||||
| [lint.pyupgrade] | ||||||
| keep-runtime-typing = true | ||||||
|
|
||||||
| [format] | ||||||
| # per default like Black | ||||||
| #preview = true | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,131 @@ | ||
| # Change Log | ||
|
|
||
| ## [[email protected]](https://github.com/amperser/proselint/compare/0.13.0...0.15.0) | ||
|
|
||
| ### Biggest Changes | ||
|
|
||
| - featureset of py38 | ||
| - cleaner and faster code | ||
| - fixed bugs, undefined behavior, broken legacy code | ||
| - some checks did not work as intended | ||
| - multiprocessed parallelization and other optimizations | ||
|
|
||
| ### Benchmark-Comparison | ||
|
|
||
| - custom set of files, same hardware | ||
|
|
||
| #### Windows | ||
|
|
||
| Proselint-main (uncached & cached) | ||
|
|
||
| - Found 104 lint-warnings in 47.579 s | ||
| - Found 104 lint-warnings in 0.878 s | ||
|
|
||
| Proselint-modernized | ||
|
|
||
| - Found 108 lint-warnings in 39.930 s (12 files, 617.45 kiByte) -> serialized | ||
| - Found 108 lint-warnings in 13.164 s (12 files, 617.45 kiByte) -> serial files, parallel checks | ||
| - Found 108 lint-warnings in 9.931 s (12 files, 617.45 kiByte) -> global check-executor | ||
| - Found 108 lint-warnings in 0.011 s (12 files, 617.45 kiByte) -> cached | ||
|
|
||
| #### Linux / WSL | ||
|
|
||
| Proselint-main (uncached & cached) | ||
|
|
||
| - Found 104 lint-warnings in 37.041 s | ||
| - Found 104 lint-warnings in 0.771 s | ||
|
|
||
| Proselint-modernized | ||
|
|
||
| - Found 108 lint-warnings in 34.521 s (12 files, 617.45 kiByte) | ||
| - Found 108 lint-warnings in 8.248 s (12 files, 617.45 kiByte) | ||
| - Found 108 lint-warnings in 6.098 s (12 files, 617.45 kiByte) | ||
| - Found 108 lint-warnings in 0.044 s (12 files, 617.45 kiByte) | ||
|
|
||
| ### Breaking changes | ||
|
|
||
| - cli arg `--time` -> `--benchmark`, `-b` | ||
| - cli arg `--debug` -> `--verbose`, `-v` | ||
| - exit code now returns number of errors | ||
| - cli arg `--output-format` controls old flags json, compact | ||
| - py >= 3.8 | ||
| - api-changes -> web_scripts & plugins untested | ||
| - config adjustments (1 test removed, 1 added) | ||
| - "misc.metaconcepts": False, # TODO: remove, was duplicate of scare_quotes | ||
|
|
||
| ### Whats broken / untested | ||
|
|
||
| - web_scripts | ||
| - plugins | ||
| - Github action | ||
|
|
||
|
|
||
| ### Detailed Changelog | ||
|
|
||
| - featureset of py38 | ||
| - use pathlib instead of string based paths | ||
| - add type-hinting | ||
| - replaced memoizer, short-comings: | ||
| - shelves have a good interface but are slow | ||
| - cache-init and -closing was a mess | ||
| - clearing cache resulted in broken states | ||
| - memoize-wrapper was wasting resources (hash of text recalc for every check, ..) | ||
| - age of cache-entries was not considered | ||
| - every cached fn had its own file-cache | ||
| - cache migration was done on every memoize-decoration | ||
| - cache | ||
| - hash test beforehand only once | ||
| - consider age of items | ||
| - memoize lint() instead of checks | ||
| - fix bugs related to: | ||
| - overshadowing builtin names (list, ...) | ||
| - string based path-traversal | ||
| - mix of relative and absolute paths (also relics that deleted file somewhere) | ||
| - varius small bugs (ie. missing comma in element lists) | ||
| - cache was not cleaned up by fn (as it should) | ||
| - lots of modernization from old codebase | ||
| - latest python versions had trouble with proselint | ||
| - update deps | ||
| - reformat using black-style, done by ruff | ||
| - linted codebase with ruff | ||
| - add logger and refactor print-output | ||
| - linter informs about duration | ||
| - results include link to file | ||
| - inform about duration, files scanned and text-size | ||
| - refactor unittest | ||
| - remove classes / boilerplate | ||
| - move tests for checks into separate dir | ||
| - more helpful error-messages | ||
| - remove duplicates | ||
| - repair broken tests | ||
| - lower complexity of tests (each test should only test for one thing) | ||
| - add unittests | ||
| - detect missing check-flags in default-config | ||
| - find unavaible checks for default-config-flags | ||
| - don't fail on wrong config-flags | ||
| - check working cache, speed-test | ||
| - check working cache, same results | ||
| - minimal changes to outer api | ||
| - config | ||
| - correctness of default-config is tested | ||
| - don't fail on faulty config-flags | ||
| - checks | ||
| - remove duplicates | ||
| - fix padding for checks using regex | ||
| - existence_check() - join and padding -> cleared up | ||
| - join is always done | ||
| - padding can be selected py Enum, defaults to separate in text | ||
| - fix checks accordingly | ||
| - put root-scripts into sep dir | ||
| - exithandler for more graceful ctrl+c | ||
| - ppm-wrapper is now more forgiving with small sample-sizes | ||
| - add parallelization | ||
| - checks() are run multiprocessed | ||
| - one global executioner collects tasks for all files to analyze | ||
| - break up the slowest checks to balance the load | ||
| - optimizations were done with profiling and benchmarking | ||
| - overhaul controling output-format by cli | ||
|
|
||
| ## [[email protected]](https://github.com/amperser/proselint/compare/0.12.0...0.13.0) | ||
|
|
||
| ### Bug Fixes | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| include README.md LICENSE.md CHANGELOG.md tests/illegal-chars.txt tests/*.py tests/test_gmeu/*.py |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| web: gunicorn app:app --log-file=- | ||
| worker: python scripts/worker.py | ||
| clock: python scripts/clock.py |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
can we at least compromise on 88 chars, as it is widely used by black? 80 chars are a relic from the 80's a bit of from the current sweetspot.
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.
88 is a non-standard line length. Sure, 80 characters is an old requirement, but I find you can pretty much always achieve what you need to and it forces you to keep things clean.
It's personal preference, I suppose, but since I refactored proselint the first time it has used 80 character line lengths.
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.
black is the current standard (as it is widely used) in python world - even ruff adopted it and formats the same :)
but ok
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.
Black's completely arbitrary, sarcastically-justified 88-character line length is one of its more controversial "opinions", though.
Particularly as it violates PEP8 — an actual standard, not (at best) a "de facto standard" like Black. (Though PEP8 admittedly did itself no favors by specifying 79 characters instead of 80, something that was met with almost immediate, near-universal rejection.)
IIRC line length was also the very first configuration option Black gained, once its original vision of being completely non-configurable collided headfirst with reality.