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

BUG - mypy should only be run over our actual code #710

Closed
greglucas opened this issue Jul 24, 2024 · 7 comments · Fixed by #731
Closed

BUG - mypy should only be run over our actual code #710

greglucas opened this issue Jul 24, 2024 · 7 comments · Fixed by #731
Assignees
Labels
bug Something isn't working Student Tasks suitable for student work

Comments

@greglucas
Copy link
Collaborator

Description of the issue

Currently, we run mypy in the current working directory:

args: [ ., --strict, --explicit-package-bases,
--disable-error-code, import-untyped,
--disable-error-code, import-not-found,
--disable-error-code, no-untyped-call,
--disable-error-code, type-arg ]

If someone adds an extra file without type hints like temp.py as a testing script, or puts their virtual environment in the current directory then mypy will try to run over those files as well.

Steps to reproduce the issue

Create a temporary file or virtual environment without type hints in your local directory, and run mypy . and it will fail.

Expected behavior (What should happen)

Only run over the project directory imap_processing/.

Actual behavior (What does happen)

venv3.12/bin/jp.py:12: error: Function is missing a return type annotation [no-untyped-def]
venv3.12/bin/rst2odt_prepstyles.py:29: error: Function is missing a type annotation [no-untyped-def]

Code Snippet:

Code

Additional notes, affected areas, and suggested fixes

Changing mypy to run only over imap_processing/. I also suggest adding the current mypy arguments from the pre-commit hook into the pyproject.toml configuration file instead. That way anyone could run mypy and get the proper ignores/additions without needing to add command line arguments.

@greglucas greglucas added the bug Something isn't working label Jul 24, 2024
@daralynnrhode
Copy link
Contributor

@greglucas From my understanding, the toml and precommit set up is strange. From what I can find, I don't think it is possible to disable the errors listed in the pre-commit in the toml file. The errors that are allowed to be disabled in the toml are broader more general errors that could include the specific errors in pre-commit, but they would also include other errors that we would want to check.
I do believe all of these can disabled from the command line, though, I'm still messing around with it.

@greglucas
Copy link
Collaborator Author

greglucas commented Jul 24, 2024

It may not be a one-to-one mapping so you might not be able to move all of them (maybe some of them aren't needed anymore even?). But a few of them I think could be moved over.

--explicit-package-bases: https://mypy.readthedocs.io/en/stable/config_file.html#confval-explicit_package_bases
https://mypy.readthedocs.io/en/stable/config_file.html#example-pyproject-toml

And then, maybe you can add all of the others to a comma separated list?
https://mypy.readthedocs.io/en/stable/config_file.html#confval-disable_error_code

@bourque bourque added the Student Tasks suitable for student work label Jul 24, 2024
@bourque bourque added this to IMAP Jul 24, 2024
@daralynnrhode
Copy link
Contributor

daralynnrhode commented Jul 24, 2024

I think I have the errors figured out and excluded from both call types, pre-commit run --all and mypy . but each run produces different errors that in no way overlap. So I still have to figure out where the pyproject and config and diverging.

*I now have the 'mypy .' producing the same error plus other errors, So there is now overlap, but I am still trying to figure out why 'mypy .' is producing more errors.

@greglucas Just to clarify, when you say you want it to run over the imap_processing directory, do you mean the repo folder imap_processing or imap_processing/imap_processing with would exclude the tools folder as well as the docs?

And should this be applied to just the pre-commit run --all or both? Because I don't think the pyproject for mypy . will allow it since passingmypy .will override any clauses in the pyproject. So instead of mypy . it should be mypy imap_processing in order to run mypy over the intended folders.

@greglucas
Copy link
Collaborator Author

@greglucas Just to clarify, when you say you want it to run over the imap_processing directory, do you mean the repo folder imap_processing or imap_processing/imap_processing with would exclude the tools folder as well as the docs?

Ahh, yes, good call on other locations if we want to check those, mostly I was getting at "it would be nice if it wasn't a blanket . check".

I think you could use file = ['imap_processing', 'docs', 'tools'] or for whatever directories you also wanted to include.

@daralynnrhode
Copy link
Contributor

I assume you are talking about the instance where pre-commits are ran and not just the individual mypy check?

@greglucas
Copy link
Collaborator Author

I assume you are talking about the instance where pre-commits are ran and not just the individual mypy check?

I'm not sure I totally follow the question, so let me know if I'm not addressing the question. I would expect the pre-commit check to run the command mypy without any extra arguments if possible, that way a user would only have to run mypy locally and get the same settings that pre-commit is using (but without having to know the extra command line arguments).

@daralynnrhode
Copy link
Contributor

Ah yes! So I am confident that I can configure the pre-commit hook to run over only the desired directories mentioned above without any extra commands. This would be done in the args section of the hook.
Currently, I don't think there is a way to apply this to when a user only runs a mypy check. Mypy when ran by itself has to have module to check passed in with it. So mypy [file or directory to check] instead of just mypy. From what I have figured out, command line arguments take precedence over any configuration file. So even if it is stated in the pyproject.toml to only check certain folders, the command line arg would take precedence.
Does that make sense? The mypy documentation is a little hard to digest so I may also just be missing something but from my current research that is what I have concluded.

@bourque bourque moved this to In Progress in IMAP Aug 1, 2024
@bourque bourque moved this to In Progress in Release v0.5.x Aug 8, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Release v0.5.x Aug 9, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in IMAP Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Student Tasks suitable for student work
Projects
Status: Done
Status: Done
3 participants