setuptools icon indicating copy to clipboard operation
setuptools copied to clipboard

Log filenames when running pytest-mypy

Open Avasam opened this issue 1 year ago • 5 comments

Summary of changes

Reference: https://github.com/realpython/pytest-mypy/pull/93 (please let me know whether you prefer relative or absolute filepaths)

Before: https://github.com/pypa/setuptools/actions/runs/10030930044/job/27720796247?pr=4352#step:9:66

_________________________ setuptools/command/sdist.py __________________________
[gw2] darwin -- Python 3.12.4 /Users/runner/work/setuptools/setuptools/.tox/py/bin/python
46: error: Need type annotation for "negative_opt" (hint: "negative_opt: dict[<type>, <type>] = ...")  [var-annotated]
________________________ setuptools/config/setupcfg.py _________________________
[gw2] darwin -- Python 3.12.4 /Users/runner/work/setuptools/setuptools/.tox/py/bin/python
111: error: Unused "type: ignore" comment  [unused-ignore]
_____________________________ setuptools/errors.py _____________________________
[gw2] darwin -- Python 3.12.4 /Users/runner/work/setuptools/setuptools/.tox/py/bin/python
32: error: Variable "setuptools.errors.OptionError" is not valid as a type  [valid-type]
32: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
32: error: Invalid base class "OptionError"  [misc]
36: error: Variable "setuptools.errors.OptionError" is not valid as a type  [valid-type]
36: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
36: error: Invalid base class "OptionError"  [misc]
40: error: Variable "setuptools.errors.BaseError" is not valid as a type  [valid-type]
40: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
40: error: Invalid base class "BaseError"  [misc]
50: error: Variable "setuptools.errors.BaseError" is not valid as a type  [valid-type]
50: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
50: error: Invalid base class "BaseError"  [misc]
_______________________ setuptools/command/build_ext.py ________________________
[gw1] darwin -- Python 3.12.4 /Users/runner/work/setuptools/setuptools/.tox/py/bin/python
33: error: Unused "type: ignore" comment  [unused-ignore]

After (relative path): https://github.com/pypa/setuptools/actions/runs/10030863175/job/27720645004?pr=4352#step:9:67

________________________ setuptools/config/setupcfg.py _________________________
[gw3] linux -- Python 3.12.4 /home/runner/work/setuptools/setuptools/.tox/py/bin/python
setuptools/config/setupcfg.py:111: error: Unused "type: ignore" comment  [unused-ignore]
_______________________ setuptools/command/build_ext.py ________________________
[gw2] linux -- Python 3.12.4 /home/runner/work/setuptools/setuptools/.tox/py/bin/python
setuptools/command/build_ext.py:33: error: Unused "type: ignore" comment  [unused-ignore]
_____________________________ setuptools/errors.py _____________________________
[gw3] linux -- Python 3.12.4 /home/runner/work/setuptools/setuptools/.tox/py/bin/python
setuptools/errors.py:32: error: Variable "setuptools.errors.OptionError" is not valid as a type  [valid-type]
setuptools/errors.py:32: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
setuptools/errors.py:32: error: Invalid base class "OptionError"  [misc]
setuptools/errors.py:36: error: Variable "setuptools.errors.OptionError" is not valid as a type  [valid-type]
setuptools/errors.py:36: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
setuptools/errors.py:36: error: Invalid base class "OptionError"  [misc]
setuptools/errors.py:40: error: Variable "setuptools.errors.BaseError" is not valid as a type  [valid-type]
setuptools/errors.py:40: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
setuptools/errors.py:40: error: Invalid base class "BaseError"  [misc]
setuptools/errors.py:50: error: Variable "setuptools.errors.BaseError" is not valid as a type  [valid-type]
setuptools/errors.py:50: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
setuptools/errors.py:50: error: Invalid base class "BaseError"  [misc]

After (absolute path): https://github.com/pypa/setuptools/actions/runs/10030898174/job/27720720510?pr=4352#step:9:66

_________________________ setuptools/command/sdist.py __________________________
[gw2] darwin -- Python 3.12.4 /Users/runner/work/setuptools/setuptools/.tox/py/bin/python
/Users/runner/work/setuptools/setuptools/setuptools/command/sdist.py:46: error: Need type annotation for "negative_opt" (hint: "negative_opt: dict[<type>, <type>] = ...")  [var-annotated]
________________________ setuptools/config/setupcfg.py _________________________
[gw2] darwin -- Python 3.12.4 /Users/runner/work/setuptools/setuptools/.tox/py/bin/python
/Users/runner/work/setuptools/setuptools/setuptools/config/setupcfg.py:111: error: Unused "type: ignore" comment  [unused-ignore]
_____________________________ setuptools/errors.py _____________________________
[gw2] darwin -- Python 3.12.4 /Users/runner/work/setuptools/setuptools/.tox/py/bin/python
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:32: error: Variable "setuptools.errors.OptionError" is not valid as a type  [valid-type]
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:32: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:32: error: Invalid base class "OptionError"  [misc]
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:36: error: Variable "setuptools.errors.OptionError" is not valid as a type  [valid-type]
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:36: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:36: error: Invalid base class "OptionError"  [misc]
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:40: error: Variable "setuptools.errors.BaseError" is not valid as a type  [valid-type]
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:40: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:40: error: Invalid base class "BaseError"  [misc]
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:50: error: Variable "setuptools.errors.BaseError" is not valid as a type  [valid-type]
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:50: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
/Users/runner/work/setuptools/setuptools/setuptools/errors.py:50: error: Invalid base class "BaseError"  [misc]

Pull Request Checklist

  • [x] Changes have tests (these are updated tests, see before & after)
  • [x] News fragment added in newsfragments/. (not user facing) (See documentation for details)

Avasam avatar Jul 21 '24 19:07 Avasam

@jaraco if this is accepted, should a conftest.py be added to the skeleton ? Also maybe this should even be implemented in jaraco.test ?

Avasam avatar Jul 30 '24 15:07 Avasam

I kind-of like the "before" output, given that the filename is given in the header. Adding more details to the output makes it more likely that the most important detail is wrapped or off-screen.

Can you tell me more about the motivation for the change?

If it's a good practice or a best practice, why wouldn't pytest-mypy apply this behavior by default? I tend to try to align with the defaults from upstream and avoid customizing downstream, except where that customization is essentially needed, or documented by the upstream project as "recommended" boilerplate.

if this is accepted, should a conftest.py be added to the skeleton ? Also maybe this should even be implemented in jaraco.test ?

I'd like to avoid adding a conftest.py to the skeleton, because it's an unstructured format and will conflict easily with downstream projects. I'd be much more inclined to implement it as a plugin, such as jaraco.test.

jaraco avatar Aug 01 '24 00:08 jaraco

Can you tell me more about the motivation for the change?

I unironically didn't think to look at the error section header, maybe I was tunnel-visioning hard that day. I think I saw the .EXE path and mentally stopped there.

I do have one functional use that's pretty standard across checker tools that I use: it's convenient to copy setuptools/errors.py:32 so my editor's "go to" command can directly go to the right line. When the test is run in an editor's shell (Vscode, PyCharm) it also allows clicking the link bringing directly to the right line.

image

It's a small convenience now that I noticed the header (which I feel silly about not noticing earlier). Although one I still have a preference for.

why wouldn't pytest-mypy apply this behavior by default?

Given the filename is in the header, I can only assume they figured it's a good enough minimal default and left any additional information to custom formatter. (ie: let users use the generalized customizable system, rather than adding new configurations)

Avasam avatar Aug 01 '24 01:08 Avasam

I agree absolute paths seem like the worst options of the three. I only included it for completeness sake.

That sounds like a good feature for a plugin or for the plugin to implement as an option.

Given your project setup for skeleton-based projects, which I'm getting more and more accustomed to, I also agree that the logic for this configuration would better live outside setuptools, with a minimal change to setuptools itself (preferably a config)

https://github.com/realpython/pytest-mypy/issues/90 was closed by https://github.com/realpython/pytest-mypy/pull/93 but I don't see any discussion saying that they explicitly wouldn't want to add it as an option.

Avasam avatar Aug 29 '24 16:08 Avasam

Converting to draft since the solution should go outside setuptools (either https://github.com/jaraco/jaraco.test, https://github.com/jaraco/jaraco.develop, or https://github.com/realpython/pytest-mypy)

Avasam avatar Sep 12 '24 19:09 Avasam

This should now be doable using --mypy-report-style=mypy from https://github.com/realpython/pytest-mypy/pull/193 I'd also pin pytest-mypy >= 1.0.1 for https://github.com/realpython/pytest-mypy/pull/195

Avasam avatar Apr 03 '25 02:04 Avasam

I've updated the PR description and comparions. But this should go in skeleton.

Avasam avatar May 15 '25 15:05 Avasam

Perhaps this change should go in pytest-enabler so it applies to any project with mypy installed?

jaraco avatar May 15 '25 23:05 jaraco

Going deep in the layered setup ^^ But yeah, that looks like the place to add this from what I can see.

I think if mypy isn't enabled, then it's possible pytest would fail with "unknown command line argument"

Avasam avatar May 16 '25 00:05 Avasam

That's the magic of pytest-enabler - it only passes the command line if the plugin is installed and enabled.

jaraco avatar May 16 '25 01:05 jaraco