mypy icon indicating copy to clipboard operation
mypy copied to clipboard

More helpful type guards

Open A5rocks opened this issue 1 year ago • 12 comments

A TODO I found while looking at something else got me down this rabbit hole, and I noticed mypy's subpar performance in these two cases.

Short descriptions from before I did this, so may be technically inaccurate:

  1. check when defining a type guard whether the function is right or not
  2. follow kw args if they go to a regular positional first arg

~~This may fix an issue but I have not conducted any searches.~~ Fixes #13199 Refs #14425

A5rocks avatar Dec 03 '22 05:12 A5rocks

Diff from mypy_primer, showing the effect of this PR on open source code:

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:119: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:199: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:204: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:303: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:307: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

github-actions[bot] avatar Dec 03 '22 06:12 github-actions[bot]

mypy primer results seem to be narrowing on self:

    @abstractmethod
    def is_error(self) -> TypeGuard[Error[_TSource, _TError]]:
        """Returns `True` if the result is an `Error` value."""

        raise NotImplementedError

This seems... wrong? Firstly, the PEP says this (as mentioned in one of my commit messages):

If a type guard function is implemented as an instance method or class method, the first positional argument maps to the second parameter (after “self” or “cls”).

This was even explicitly rejected! https://peps.python.org/pep-0647/#narrowing-of-implicit-self-and-cls-parameters

Secondly, mypy doesn't even support this!! At the moment:

from typing import TypeGuard

class A:
  def n(self) -> TypeGuard[int]:
    return False

a = A()
if a.n():
  reveal_type(a)  # N: Revealed type is "__main__.A"

However, I realize I missed staticmethod special casing. Will fix.

from typing_extensions import TypeGuard

class Z:
    @staticmethod
    def typeguard(h: object) -> TypeGuard[int]:  # E: TypeGuard functions must have a positional argument
        return isinstance(h, int)

x: object
if Z().typeguard(x):
    reveal_type(x)
if Z.typeguard(x):
    reveal_type(x)

A5rocks avatar Dec 03 '22 06:12 A5rocks

Diff from mypy_primer, showing the effect of this PR on open source code:

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:119: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:199: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:204: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:303: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:307: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

github-actions[bot] avatar Dec 03 '22 07:12 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:119: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:199: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:204: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:303: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:307: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

github-actions[bot] avatar Dec 05 '22 14:12 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:131: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:212: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:217: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:323: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:327: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

github-actions[bot] avatar Dec 08 '22 05:12 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:131: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:212: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:217: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:323: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:327: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

github-actions[bot] avatar Dec 10 '22 06:12 github-actions[bot]

OK with that tests should be added as requested. I've got one additional question that I've moved over to #14273 because it may cause more debate. Hopefully this as it stands is alright!

A5rocks avatar Dec 10 '22 06:12 A5rocks

Diff from mypy_primer, showing the effect of this PR on open source code:

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:131: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:212: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:217: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:323: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:327: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

github-actions[bot] avatar Dec 10 '22 06:12 github-actions[bot]

Diff from mypy_primer, showing the effect of this PR on open source code:

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:131: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:212: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:217: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:323: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:327: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

github-actions[bot] avatar Dec 11 '22 06:12 github-actions[bot]

Cases that error:

   
    @abstractmethod
    def is_error(self) -> TypeGuard[Error[_TSource, _TError]]:
        """Returns `True` if the result is an `Error` value."""

        raise NotImplementedError

    @abstractmethod
    def is_ok(self) -> TypeGuard[Ok[_TSource, _TError]]:
        """Returns `True` if the result is an `Ok` value."""

        raise NotImplementedError

sobolevn avatar Dec 11 '22 07:12 sobolevn

Yep! As mentioned before, that is invalid as according to the PEP and mypy doesn't support it anyways. Having an error at definition time is likely beneficial.

A5rocks avatar Dec 11 '22 07:12 A5rocks

Someone ran into this in the issue tracker; I have updated the "fixes" section.

A5rocks avatar Jan 12 '23 05:01 A5rocks

Diff from mypy_primer, showing the effect of this PR on open source code:

Tanjun (https://github.com/FasterSpeeding/Tanjun)
- version: 1.0.0+dev.7c14ebae6ca5d6ec39600e122b58a62afaa3ab02
+ version: 1.0.0+dev.d490f40dcc4f6244b2c486e9f6b6a7234763310f
-   File "/semanal.py", line 6235, in accept
+   File "/semanal.py", line 6249, in accept
-   File "/semanal.py", line 5909, in defer
+   File "/semanal.py", line 5923, in defer

Expression (https://github.com/cognitedata/Expression)
+ expression/core/result.py:125: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:131: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:212: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:217: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:323: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/result.py:327: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:139: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:144: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:214: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:218: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:330: error: TypeGuard functions must have a positional argument  [valid-type]
+ expression/core/option.py:334: error: TypeGuard functions must have a positional argument  [valid-type]

github-actions[bot] avatar Jan 31 '23 06:01 github-actions[bot]

Oh hmm, the crash on FasterSpeeding/Tanjun is new (not to this PR, just new in general)

hauntsaninja avatar Jan 31 '23 07:01 hauntsaninja