Skip to content

Add --mypy-report-style #193

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

Merged
merged 2 commits into from
Mar 28, 2025
Merged

Add --mypy-report-style #193

merged 2 commits into from
Mar 28, 2025

Conversation

dmtucker
Copy link
Collaborator

@dmtucker dmtucker commented Mar 14, 2025

Resolve #90

$ venv/bin/pytest --mypy-report-style no-path demo/bad.py 
====================== test session starts =======================
platform linux -- Python 3.10.12, pytest-8.3.5, pluggy-1.5.0
Using --randomly-seed=827345869
rootdir: /home/dtux/Projects/pytest-mypy
configfile: tox.ini
plugins: mypy-0.10.4.dev92+g7e232a1, cov-4.1.0, randomly-3.16.0
collected 2 items                                                

demo/bad.py FF                                             [100%]

============================ FAILURES ============================
_______________________ [mypy] demo/bad.py _______________________
1: error: Incompatible types in assignment (expression has type "str", variable has type "int")  [assignment]
__________________________ test session __________________________
mypy exited with status 1.
============================== mypy ==============================
Found 1 error in 1 file (checked 1 source file)
==================== short test summary info =====================
FAILED demo/bad.py::mypy1.15.0
FAILED demo/bad.py::mypy1.15.0-status
======================= 2 failed in 0.27s ========================
  • Note: no-path style is still the default.
$ venv/bin/pytest --mypy-report-style mypy demo/bad.py 
====================== test session starts =======================
platform linux -- Python 3.10.12, pytest-8.3.5, pluggy-1.5.0
Using --randomly-seed=373067107
rootdir: /home/dtux/Projects/pytest-mypy
configfile: tox.ini
plugins: mypy-0.10.4.dev92+g7e232a1, cov-4.1.0, randomly-3.16.0
collected 2 items                                                

demo/bad.py FF                                             [100%]

============================ FAILURES ============================
__________________________ test session __________________________
mypy exited with status 1.
_______________________ [mypy] demo/bad.py _______________________
demo/bad.py:1: error: Incompatible types in assignment (expression has type "str", variable has type "int")  [assignment]
============================== mypy ==============================
Found 1 error in 1 file (checked 1 source file)
==================== short test summary info =====================
FAILED demo/bad.py::mypy1.15.0-status
FAILED demo/bad.py::mypy1.15.0
======================= 2 failed in 0.27s ========================
  • mypy style returns the lines unaltered (i.e. exactly how mypy created them).

@dmtucker dmtucker marked this pull request as ready for review March 16, 2025 22:14
@jaraco
Copy link

jaraco commented Mar 18, 2025

Thanks for the proposal!

--mypy-default-report-style {mypy,pytest}

Can I nitpick the naming a bit? I don't see how "default" is meaningful here. Oh, I think I see. It's the default style if it's not further customized? That feels like an unlikely scenario. I'd suggest just mypy-report-style or maybe mypy-report-path.

As for "mypy" and "pytest" - those also feel a little disconnected from the user's intent. A user unfamiliar with this pull request or linked issue is going to be unlikely to know what "mypy" means as a report style, same for "pytest". In fact, the "pytest" style is a bit of a misnomer as the pytest convention in other tools is to report the relative path in the message (I think). I'd rather see something closer to the user's intent, like "inline"/"hidden" or "show"/"hide" or "quiet"/"verbose".

@dmtucker
Copy link
Collaborator Author

It's the default style if it's not further customized?

Exactly... This controls the behavior of default_file_error_formatter (not file_error_formatter, which can be changed).
I don't love the extra verbosity, but I do think it's the most accurate description 😕

I suppose I could live with --mypy-report-style, but I think --mypy-report-path is a little too specific.

A user unfamiliar with this pull request or linked issue is going to be unlikely to know what "mypy" means as a report style, same for "pytest".

That's fair. I think we should address this with documentation (e.g. additional help text).
That will better capture the intent of these styles too:

  • pytest: Report in a way that preserves the spirit of the default Pytest output (i.e. adapt the mypy output as much as possible).
  • mypy: Report in a way that will feel most familiar to someone who just runs mypy directly (i.e. modify the mypy output as little as possible).

In fact, the "pytest" style is a bit of a misnomer as the pytest convention in other tools is to report the relative path in the message (I think).

Note that I wasn't really going for "the style that other plugins which integrate static checkers use" as much as 'the style that core uses".

Also, for what it's worth, I have issue #23 in the back of my mind for the pytest style... When a regular test fails, you don't usually just get a one-line error mentioning the line number and description, you get assertion-rewriting that brings the affected code to you. I think that could be cool to see that at some point, perhaps even as the default style, if done well.

@dmtucker
Copy link
Collaborator Author

a little disconnected from the user's intent

I think that's a good point, particularly for the "pytest" name, so I ended up renaming it "no-path".
That also makes it clearer if issue #23, etc. introduces a new style.

@dmtucker dmtucker changed the title Add --mypy-default-report-style Add --mypy-report-style Mar 27, 2025
@dmtucker
Copy link
Collaborator Author

Oops... helps if I actually commit the changes 😅

@dmtucker dmtucker merged commit 66eab40 into realpython:main Mar 28, 2025
6 checks passed
@dmtucker dmtucker deleted the reporting branch March 28, 2025 04:00
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.

Don't strip filename from errors
2 participants