Setup mypy for regression checks #684

Dreamsorcerer · 2022-01-23T14:59:00Z

Multidict 6.0.0 and 6.0.1 have messed up typing.

This adds type checking to atleast the test_multidict.py file to check for regressions in future.

Still needs regressions to be fixed though, in order for the test to pass.

asvetlov

Thanks for the PR.
The current master (and 6.0.2) uses a different approach: it enumerates all possible calls in tests/test_mypy.py and runs it in --strict mode.

Adding strict annotations to other tests (and maybe multidict/_*.py files) makes sense to me. Please feel free to update the PR in this manner.
If you have no capacity to convert all files -- it's fine, please fix the merge conflict at least.

Dreamsorcerer · 2022-01-24T17:33:08Z

Thanks for the PR. The current master (and 6.0.2) uses a different approach: it enumerates all possible calls in tests/test_mypy.py and runs it in --strict mode.

I'll merge them together and we can run on both files.

Adding strict annotations to other tests (and maybe multidict/_*.py files) makes sense to me. Please feel free to update the PR in this manner. If you have no capacity to convert all files -- it's fine, please fix the merge conflict at least.

The multidict/_*.py files currently have no annotations, as everything is in the .pyi file, which is why I've left them alone. The original mypy invocation doesn't actually do anything as it will only check annotated functions.

.mypy.ini

codecov · 2022-01-24T20:17:40Z

Codecov Report

Merging #684 (6953b18) into master (10d18dd) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #684   +/-   ##
=======================================
  Coverage   93.65%   93.65%           
=======================================
  Files           5        5           
  Lines         504      504           
=======================================
  Hits          472      472           
  Misses         32       32

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

webknjaz · 2022-09-13T23:43:20Z

.mypy.ini

+follow_imports_for_stubs = True
+implicit_reexport = False
+no_implicit_optional = True
+show_error_codes = True


also show line numbers?

That's not a config option. :P
It always shows line numbers, otherwise it'd be pretty useless..

@Dreamsorcerer right, I confused that with show_column_numbers = true.
I've also learned about these: pretty = true, color_output = True, error_summary = True. Here's one config example with some of my findings: https://github.com/sphinx-contrib/sphinxcontrib-towncrier/blob/master/mypy.ini.

Another thing I discovered is a text report. Here's an example: https://github.com/sphinx-contrib/sphinxcontrib-towncrier/blob/9e84d94/.pre-commit-config.yaml#L181.

Mind looking through these ideas and implementing them here or in a follow-up?

Also, looks like with python/mypy#8192, there's a possibility to have strict = True in the config now (additionally, not instead of the individual toggles).

@Dreamsorcerer I learned that you can also have enable_error_code = ["ignore-without-code"] to prevent wholesale ignores.

color_output, error_summary default to True (not that I've ever seen any colour in the output...

Would be really useful if we could have a global mypy config shared between all our repos with local overrides...

color_output, error_summary default to True (not that I've ever seen any colour in the output...

@Dreamsorcerer many tools have TTY auto-detection and use plain text output in non-interactive invocations. OTOH, most CIs support rendering ANSI colored terminal output. GHA runners don't allocate a TTY for job invocations (some other CIs do this). This means that all tools that don't special-case the CIs (which can be detected via env vars), don't produce ANSI output to be on the safe side. A lot of these apps have a CLI flag to force the color output anyway. And there's a conventional env var that is gaining popularity over the past years for doing the same — $FORCE_COLOR. If it is set, it is expected that a program supporting it would produce colorized output regardless of the presence of a TTY. Its popular counterpart $NO_COLOR even has its own website — https://no-color.org. Also, some software that doesn't yet support FORCE_COLOR (like old MyPy), has own env vars (e.g. MYPY_FORCE_COLOR).

As you can see, there's a number of reasons you haven't seen the colored output, especially if you're mostly checking out the CI logs.
Here's what I've been recently integrating into CIs, having realized that most of my pytest and mypy runs didn't produce fancy logs: jaraco/skeleton@e22ae08 (#67).

Back to the color_output setting — I'm going to guess it may only default to true in certain envs, but not the others. So adding it would still be warranted. Alternatively (or additionally), it'd be nice to have that patch I linked above integrated too.

Not sure about error_summary but sometimes I just follow the principle of explicit being better than implicit. From the PoV of a config reader, this reduces the mental overhead of having to figure out what the other settings are present implicitly. Plus, this saves from surprise defaults changes on updates.

Would be really useful if we could have a global mypy config shared between all our repos with local overrides...

Yes, I've been toying (currently in my mind only) with an idea of having multiple skeletons for different aspects of projects being merged into an org-shared skeleton with typical file contents.
Something like https://github.com/jaraco/skeleton (which jaraco uses in all his projects, including setuptools and importlib-metadata, for example). But a more distributed ecosystem. Such skeletons are merged into other repositories downstream with git merge --allow-unrelated-histories (only for the first time, following update-merges shouldn't need this extra CLI flag).

As you can see, there's a number of reasons you haven't seen the colored output, especially if you're mostly checking out the CI logs.

I've not seen it in my local terminal, and I did try explicitly setting the option (git diff or pytest produce coloured output...). Weird..

python/mypy#7771 says "mypy ignores color_output=true if there is no tty" as I suspected.

Have you tried requesting it explicitly (as in MYPY_FORCE_COLOR=1 mypy)? Maybe it's still not forced somehow?
Here's somebody else's screenshot: python/mypy#7771 (comment).

P.S. Have you made sure there's actual errors? I don't remember if there's anything colored in successful runs.

This doesn't seem relevant anymore.

webknjaz

@Dreamsorcerer feel free to merge once the requests are addressed.

webknjaz · 2022-10-03T23:21:18Z

@Dreamsorcerer I think that we should get this in so that it's merged by the time we make a new release (I plan on doing this once Python 3.11 final is out).

Dreamsorcerer · 2022-10-07T17:29:36Z

Think it's blocked on the timeline protection...

webknjaz · 2022-10-16T22:10:01Z

Think it's blocked on the timeline protection...

I don't see a red cross there. It reports a neutral outcome, which is treated by the branch protection as passing. If there was to label specified, it'd block with a failed outcome.

So maybe GH was having problems itself.

webknjaz · 2022-10-16T22:10:46Z

.mypy.ini

+disallow_untyped_calls = True
+disallow_untyped_decorators = True
+disallow_untyped_defs = True
+enable_error_code = redundant-expr, truthy-bool, ignore-without-code, unused-awaitable


I wonder if this can be multilined

Dreamsorcerer added 2 commits January 23, 2022 14:56

Add mypy check for regressions.

2789c4b

Tweak mypy command

2682b5b

Dreamsorcerer requested a review from asvetlov as a code owner January 23, 2022 14:59

Line wrapping.

c85804c

Dreamsorcerer added the bot:chronographer:skip This PR does not need to include a change note label Jan 23, 2022

Dreamsorcerer added 2 commits January 23, 2022 15:04

Unused import

c8e6c04

Install dependencies for type checking.

d1b1a92

Dreamsorcerer mentioned this pull request Jan 23, 2022

Cleanup errors with new multidict aio-libs/aiohttp#6551

Closed

asvetlov previously requested changes Jan 24, 2022

View reviewed changes

Dreamsorcerer added 2 commits January 24, 2022 19:21

Merge master.

c5c7653

Fix type errors.

0d48bc1

Dreamsorcerer commented Jan 24, 2022

View reviewed changes

.mypy.ini Outdated Show resolved Hide resolved

Dreamsorcerer commented Jan 24, 2022

View reviewed changes

.mypy.ini Show resolved Hide resolved

Dreamsorcerer added 2 commits January 24, 2022 20:10

noqa

7946e86

Move comment

eff576f

webknjaz reviewed Sep 13, 2022

View reviewed changes

webknjaz mentioned this pull request Sep 22, 2022

Bump flake8-pyi from 20.10.0 to 22.8.2 #758

Merged

webknjaz assigned Dreamsorcerer Sep 22, 2022

webknjaz approved these changes Sep 22, 2022

View reviewed changes

Dreamsorcerer and others added 4 commits October 7, 2022 17:59

Merge branch 'master' into mypy

cbba281

Additional options.

947d962

Extra error codes.

16e1be8

Update lint.txt

6953b18

Dreamsorcerer enabled auto-merge (squash) October 7, 2022 17:18

Dreamsorcerer merged commit 4682dbb into master Oct 16, 2022

Dreamsorcerer deleted the mypy branch October 16, 2022 12:47

webknjaz reviewed Oct 16, 2022

View reviewed changes

Uh oh!

Setup mypy for regression checks #684

Setup mypy for regression checks #684

Uh oh!

Conversation

Dreamsorcerer commented Jan 23, 2022

Uh oh!

asvetlov left a comment

Choose a reason for hiding this comment

Uh oh!

Dreamsorcerer commented Jan 24, 2022

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webknjaz left a comment

Choose a reason for hiding this comment

Uh oh!

webknjaz commented Oct 3, 2022

Uh oh!

Dreamsorcerer commented Oct 7, 2022

Uh oh!

webknjaz commented Oct 16, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jan 24, 2022 •

edited

Loading