Skip to content
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

Apply assorted repo-review rules #191

Merged
merged 5 commits into from
Aug 8, 2024

Conversation

DimitriPapadopoulos
Copy link
Contributor

Follow-up of #187 (comment).

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Aug 7, 2024

I don't understand the MyPy errors in CI:

MyPy 1.11.1 errors in CI
typecheck: commands[0]> python -m mypy --pretty --show-error-context src
src/validate_pyproject/_tomllib.py:10: error: Unused "type: ignore" comment 
[unused-ignore]
            from toml import TomlDecodeError as TOMLDecodeError  # type: i...
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~...
src/validate_pyproject/_tomllib.py:10: error: Library stubs not installed for
"toml"  [import-untyped]
            from toml import TomlDecodeError as TOMLDecodeError  # type: i...
    ^
src/validate_pyproject/_tomllib.py:10: note: Error code "import-untyped" not covered by "type: ignore" comment
src/validate_pyproject/_tomllib.py:10: note: Hint: "python3 -m pip install types-toml"
src/validate_pyproject/_tomllib.py:10: note: (or run "mypy --install-types" to install all missing stub packages)
src/validate_pyproject/_tomllib.py:10: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
src/validate_pyproject/_tomllib.py:10: error: Name "TOMLDecodeError" already
defined (possibly by an import)  [no-redef]
            from toml import TomlDecodeError as TOMLDecodeError  # type: i...
            ^
src/validate_pyproject/_tomllib.py:10: note: Error code "no-redef" not covered by "type: ignore" comment
src/validate_pyproject/_tomllib.py:11: error: Unused "type: ignore" comment 
[unused-ignore]
            from toml import loads  # type: ignore[assignment]
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/validate_pyproject/_tomllib.py:11: error: Name "loads" already defined
(possibly by an import)  [no-redef]
            from toml import loads  # type: ignore[assignment]
            ^
src/validate_pyproject/_tomllib.py:11: note: Error code "no-redef" not covered by "type: ignore" comment
src/validate_pyproject/formats.py:86: error: Unused
"type: ignore[import-untyped]" comment  [unused-ignore]
            from setuptools._vendor.packaging import (  # type: ignore[imp...
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~...
src/validate_pyproject/formats.py:209: error: Unused "type: ignore" comment 
[unused-ignore]
        from trove_classifiers import (  # type: ignore[import-not-found]
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I cannot reproduce them locally:

$ python -m mypy --pretty --show-error-context src
Success: no issues found in 21 source files
$ 
$ mypy --version
mypy 1.11.1 (compiled: yes)
$ 

Perhaps related to pinned-down versions of dependencies?

typecheck: fastjsonschema==2.20.0,importlib_resources==6.4.0,mypy==1.11.1,mypy-extensions==1.0.0,packaging==24.1,pip==24.1,setuptools==70.1.0,tomli==2.0.1,trove-classifiers==2024.7.2,typing_extensions==4.12.2,validate-pyproject @ file:///tmp/cirrus-ci-build/dist/validate_pyproject-0.18.post1.dev14%2Bgc585ce5-py3-none-any.whl#sha256=6c3d97a42127e4c1af7f88fcf3b52101a7b3a433b7c32c04ef81f74588857fec,wheel==0.43.0

@DimitriPapadopoulos DimitriPapadopoulos force-pushed the repo-review branch 10 times, most recently from 34d2ff4 to 670ee2a Compare August 7, 2024 17:46
pyproject.toml Outdated Show resolved Hide resolved
MY104: MyPy enables ignore-without-code
Must have "ignore-without-code" in enable_error_code = [...].
This will force all skips in your project to include the error
code, which makes them more readable, and avoids skipping
something unintended.

MY105: MyPy enables redundant-expr
Must have "redundant-expr" in enable_error_code = [...].
This helps catch useless lines of code, like checking the
same condition twice.

MY106: MyPy enables truthy-bool
Must have "truthy-bool" in enable_error_code = [].
This catches mistakes in using a value as truthy if it
cannot be falsey.
Disable many rules for now, they can be enabled later on.
@abravalheri abravalheri merged commit 056262f into abravalheri:main Aug 8, 2024
27 checks passed
@abravalheri
Copy link
Owner

Thank you very much!

So with this pre-commit will autofix unused type: ignore annotations, right?

@henryiii
Copy link
Collaborator

henryiii commented Aug 8, 2024

except prettier, no plans to use nodejs in the CI for a Python project

If you know a good markdown linter or formatter alternative, happy to add it in the list of known list.

So with this pre-commit will autofix unused type: ignore annotations, right?

No, you need something that reads python files for that. There are several options, but I'm personally waiting to see what the Ruff team produces. They are working on a type checker; last I heard it was going to be part of Ruff (and the prototype is part of Ruff), though I'm curious to see if they stick with that - Ruff famously doesn't require or use any dependencies, but a type checker does need dependencies and stub files.

@abravalheri
Copy link
Owner

If you know a good markdown linter or formatter alternative, happy to add it in the list of known list.

I haven't tested mdformat yet so I cannot voucher for it, but it comes from the executable books folk, which usually do nice stuff.

@DimitriPapadopoulos
Copy link
Contributor Author

So with this pre-commit will autofix unused type: ignore annotations, right?

No, you need something that reads python files for that. There are several options, but I'm personally waiting to see what the Ruff team produces. They are working on a type checker; last I heard it was going to be part of Ruff (and the prototype is part of Ruff), though I'm curious to see if they stick with that - Ruff famously doesn't require or use any dependencies, but a type checker does need dependencies and stub files.

This PR modifies the MyPy options so that MyPy catches unused type: ignore annotations when run. However, I don't see pre-commit running MyPy; it would probably involve adding this hook: https://github.com/pre-commit/mirrors-mypy. On the other hand, one of the GitHub CI jobs does run MyPy — maybe through tox, I don't know how it's done.

@DimitriPapadopoulos DimitriPapadopoulos deleted the repo-review branch August 8, 2024 16:45
@henryiii
Copy link
Collaborator

henryiii commented Aug 8, 2024

I'm a bit worried about mdformat. It's not been touched in months. It seems to be worked on by the author of tomli, who's not make a public commit since April. It does have plugins for lots of things, but the plugin system isn't configurable; this is particularly bad when some plugins change the target (what happens if mdformat-gfm and mdformat-rtd are both installed?). Happy to add it to the list of supported formatters, but I'm probably not going to propose it as an alternative just yet.

I know people at executable books, so might ask around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants