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

PEP 724: Stricter TypeGuard #3266

Merged
merged 83 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
83 commits
Select commit Hold shift + click to select a range
f5205c6
Update after Erik's feedback
rchiodo Aug 2, 2023
9da9233
Erik's initial comments
rchiodo Aug 2, 2023
00d5247
Update after Erik's feedback
rchiodo Aug 2, 2023
79720d5
Merge branch 'rchiodo/pep-722-strictertypeguard' of https://github.co…
rchiodo Aug 2, 2023
4c4a0e9
Update pep-0722.rst
rchiodo Aug 3, 2023
4f90e9c
Add the deliberate mistake case
rchiodo Aug 3, 2023
faa5768
Merge branch 'rchiodo/pep-722-strictertypeguard' of https://github.co…
rchiodo Aug 3, 2023
f00e7a2
Update pep-0722.rst
rchiodo Aug 3, 2023
c895a0a
Update pep-0722.rst
rchiodo Aug 3, 2023
5305af4
Update pep-0722.rst
rchiodo Aug 3, 2023
b6335f0
Update pep-0722.rst
rchiodo Aug 3, 2023
f24ca7c
Update pep-0722.rst
rchiodo Aug 3, 2023
cbf907a
More review feedback
rchiodo Aug 3, 2023
dea607f
Merge branch 'rchiodo/pep-722-strictertypeguard' of https://github.co…
rchiodo Aug 3, 2023
e5a71ea
More review feedback
rchiodo Aug 3, 2023
a32efa4
Fix 80 column limit
rchiodo Aug 3, 2023
708f60c
Update pep-0722.rst
rchiodo Aug 3, 2023
662341f
Update pep-0722.rst
rchiodo Aug 3, 2023
24b88ba
More feedback
rchiodo Aug 3, 2023
a682219
Review feedback
rchiodo Aug 3, 2023
6287be5
Some more subtle word changes
rchiodo Aug 3, 2023
a2e0b22
Change verbiage a little more
rchiodo Aug 3, 2023
66bdf14
Merge pull request #1 from rchiodo/rchiodo/pep-722-strictertypeguard
rchiodo Aug 4, 2023
5850d66
Change pep number
rchiodo Aug 4, 2023
31fb9c7
Merge branch 'rchiodo/pep-722' of https://github.com/rchiodo/peps int…
rchiodo Aug 4, 2023
641b761
Update PEP number
rchiodo Aug 4, 2023
1ec2207
Fix some errors in the pre-commit
rchiodo Aug 4, 2023
ce63130
Merge branch 'main' into rchiodo/pep_725
rchiodo Aug 4, 2023
6184c24
Review feedback
rchiodo Aug 4, 2023
00e028c
Merge branch 'rchiodo/pep_725' of https://github.com/rchiodo/peps int…
rchiodo Aug 4, 2023
7ab28d9
Update codeowners
rchiodo Aug 4, 2023
dddc7cb
Remove ``Resolution``
AA-Turner Aug 4, 2023
7e51159
Move to specification
rchiodo Aug 4, 2023
68e73a3
Merge branch 'rchiodo/pep_725' of https://github.com/rchiodo/peps int…
rchiodo Aug 4, 2023
a9b0407
Update pep-0724.rst
rchiodo Aug 4, 2023
106f10a
Formatting and grammar
rchiodo Aug 4, 2023
e44d060
Merge branch 'rchiodo/pep_725' of https://github.com/rchiodo/peps int…
rchiodo Aug 4, 2023
07af8f7
Update pep-0724.rst
rchiodo Aug 4, 2023
81bc56c
More review feedback
rchiodo Aug 4, 2023
535ab81
Fix footnotes
rchiodo Aug 4, 2023
7b69948
Update based on Eric's feedback
rchiodo Aug 4, 2023
ddc4ec3
Fix build warnings
rchiodo Aug 4, 2023
dd4bf80
Fixup comment
rchiodo Aug 4, 2023
5eab931
Add some more examples
rchiodo Aug 5, 2023
c5449b8
Rework 'backwards compatibility'
rchiodo Aug 5, 2023
6ff5be9
Grammar
rchiodo Aug 5, 2023
487a47e
Remove ``Resolution``
AA-Turner Aug 5, 2023
4f9753a
Update pep-0724.rst
rchiodo Aug 7, 2023
3c37ec3
Update pep-0724.rst
rchiodo Aug 7, 2023
27bac54
Merge branch 'main' into rchiodo/pep_725
rchiodo Aug 7, 2023
b90df2c
Fix column width
rchiodo Aug 7, 2023
3263b63
Another try for backwards compatibility
rchiodo Aug 8, 2023
dcf3d49
another blurb about backwards compatibility
rchiodo Aug 8, 2023
598d0da
Merge remote-tracking branch 'upstream/main' into rchiodo/pep_725
rchiodo Aug 25, 2023
a580f4a
Add mypy primer test
rchiodo Aug 25, 2023
95d4616
Add another rejected idea
rchiodo Aug 25, 2023
b267456
Fix typo
rchiodo Aug 28, 2023
6b246f2
Update based on discussions with Eric
rchiodo Aug 30, 2023
0e24a6f
Remove unnecessary lines and incorrect statement about covariance
rchiodo Aug 31, 2023
457ea0e
Merge branch 'main' into rchiodo/pep_725
rchiodo Aug 31, 2023
5ff0408
Use 'math' to specify logic
rchiodo Sep 1, 2023
b91a5ca
Merge branch 'main' into rchiodo/pep_725
rchiodo Sep 1, 2023
d8ea2cc
Don't reference intersections as a future PEP
rchiodo Sep 1, 2023
671df1f
Extra line
rchiodo Sep 1, 2023
d3048de
Improve wording
rchiodo Sep 1, 2023
4a7039e
Move degenerate case
rchiodo Sep 5, 2023
e3fd558
Some formatting
rchiodo Sep 5, 2023
bccb5fb
Merge branch 'main' into rchiodo/pep_725
rchiodo Sep 5, 2023
e5f4749
Update pep-0724.rst
JelleZijlstra Sep 8, 2023
25fc119
Merge remote-tracking branch 'upstream/main' into rchiodo/pep_725
AA-Turner Sep 9, 2023
44a31ae
Move to ``peps/``
AA-Turner Sep 9, 2023
93b1503
Whitespace
AA-Turner Sep 9, 2023
499639f
Add trailing comma
AA-Turner Sep 9, 2023
b181ef9
Copyright formatting
AA-Turner Sep 9, 2023
5963e45
Update peps/pep-0724.rst
rchiodo Sep 11, 2023
867c541
Update peps/pep-0724.rst
rchiodo Sep 11, 2023
2a2336a
Update peps/pep-0724.rst
rchiodo Sep 11, 2023
dd5fdee
More review feedback
rchiodo Sep 11, 2023
92768e2
Updates from Eric Traut
rchiodo Sep 15, 2023
159c519
Fix build and linter
rchiodo Sep 15, 2023
8f28e7a
Merge branch 'main' into rchiodo/pep_725
rchiodo Sep 15, 2023
678ce5f
Try fixing post history
rchiodo Sep 15, 2023
f817e07
Fix post history again
rchiodo Sep 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ pep-0719.rst @Yhg1s
pep-0720.rst @FFY00
pep-0721.rst @encukou
pep-0722.rst @pfmoore
pep-0725.rst TBD
rchiodo marked this conversation as resolved.
Show resolved Hide resolved
# ...
# pep-0754.txt
# ...
Expand Down
236 changes: 236 additions & 0 deletions pep-0725.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
PEP: 725
Title: Stricter Type Guards
Author: Rich Chiodo <rchiodo at microsoft.com>, Eric Traut <erictr at microsoft.com>, Erik De Bonte <erikd at microsoft.com>
Sponsor: <real name of sponsor>
PEP-Delegate: <PEP delegate's real name>
Discussions-To: https://mail.python.org/archives/list/[email protected]/thread/EMUD2D424OI53DCWQ4H5L6SJD2IXBHUL/
Status: Draft
Type: Standards Track
Topic: Typing
Content-Type: text/x-rst
Created: 28-Jul-2023
Python-Version: 3.10
rchiodo marked this conversation as resolved.
Show resolved Hide resolved
Post-History:
Resolution:


Abstract
========

:pep:`647` introduced the concept of ``TypeGuard`` functions which return true
if their input parameter matches their target type. For example, a function that
returns ``TypeGuard[str]`` is assumed to return ``true`` if and only if it's
input parameter is a ``str``. This allows type checkers to narrow types in this
positive case.

This PEP further refines :pep:`647` by allowing type checkers to also narrow types
when a ``TypeGuard`` function returns false.

Motivation
==========

``TypeGuards`` are used throughout Python libraries to allow a type checker to
narrow the type of something when the ``TypeGuard`` returns true.

However, in the ``else`` clause, :pep:`647` didn't prescribe what the type might
be:

.. code-block:: python

def is_str(val: str | int) -> TypeGuard[str]:
return isinstance(val, str)

def func(val: str | int):
if is_str(val):
# Type checkers can assume val is a 'str' in this branch
else:
# Type here is not narrowed. It is still 'str | int'


This PEP proposes that when the type argument of the ``TypeGuard`` is a subtype
of the type of the first input parameter, then the false return case can be
further narrowed.

This changes the example above like so:

.. code-block:: python

def is_str(val: str | int) -> TypeGuard[str]:
return isinstance(val, str)

def func(val: str | int):
if is_str(val):
# Type checkers can assume val is a 'str' in this branch
else:
# Type checkers can assume val is an 'int' in this branch

Since the ``TypeGuard`` type (or output type) is a subtype of the first input
parameter type, a type checker can determine that the only possible type in the
``else`` is the other type in the ``Union``. In this example, it is safe to
assume that if ``is_str`` returns false, then type of the ``val`` argument is an
``int``.


Specification
=============

This PEP requires no new changes to the language. It is merely modifying the
definition of ``TypeGuard`` for type checkers. Runtimes are already behaving
in this way.


Existing ``TypeGuard`` usage may change though, as described in
`Backwards Compatibility`_.

Unsafe Narrowing
--------------------

There are cases where this further type narrowing is not possible though. Here's
an example:

.. code-block:: python

def is_str_list(val: list[int | str]) -> TypeGuard[list[str]]
return all(isinstance(x, str) for x in val)

def func(val: list[int | str]):
if is_str_list(val):
# Type checker assumes list[str] here
else:
# Type checker cannot assume list[int] here

Since ``list`` is invariant, it doesn't have any subtypes. ``list[str]`` is not
rchiodo marked this conversation as resolved.
Show resolved Hide resolved
a subtype of ``list[str | int]``. This means type checkers cannot narrow the
type to ``list[int]`` in the false case.

Type checkers should not assume any narrowing in the false case when the
``TypeGuard`` type argument is not a subtype of the first input parameter type.

However, narrowing in the true case is still possible. In the example above, the
type checker can assume the list is a ``list[str]`` if the ``TypeGuard``
function returns true.

User error
--------------------------

The new ``else`` case for a ``TypeGuard`` can be setup incorrectly. Here's an
example:

.. code-block:: python

def is_positive_int(val: int | str) -> TypeGuard[int]:
return isinstance(val, int) and val > 0

def func(val: int | str):
if is_positive_int(val):
# Type checker assumes int here
else:
# Type checker assumes str here

A type checker will assume for the else case that the value is ``str``. This is
a change in behavior from :pep:`647` but as that pep stated, there are many ways
a determined or uninformed developer can subvert type safety.

A better way to handle this example would be something like so:

.. code-block:: python

PosInt = NewType('PosInt', int)

def is_positive_int(val: PosInt | int | str) -> TypeGuard[PosInt]:
return isinstance(val, int) and val > 0

def func(val: int | str):
if is_positive_int(val):
# Type checker assumes PosInt here
else:
# Type checker assumes str | int here


Backwards Compatibility
=======================

For preexisting code this PEP should require no changes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't type checker behavior change on some code? From previous GH discussion it seems that we generally think it doesn't matter in any realistic use cases, but the PEP should make an explicit argument for why it's OK to break compatibility with the previously specified behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not sure how to word that at the moment. Existing code should continue to behave the same way it does now, but yes type checkers will have different behavior. Which might cause more problems if people assume type checkers are always right. Guido's example comes to mind: python/typing#996 (comment)

Perhaps this should say something like:

`Type checkers will now be making more assumptions about the results of TypeGuard function. This can potentially cause problems like in this example:

def is_positive_int(a: int|str) -> TypeGuard[int]:
return isinstance(a, int) and a >= 0

However, there's no practical reason why anybody would assume the negative case in this situation wasn't an int. Users can still create TypeGuards that are invalid, but in the majority case, the TypeGuard will now behave in a more consistent manor.`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow completely. This PEP's only proposed change is to type checker behavior, so that's the backward compatibility we're concerned about.

Somebody who used a TypeGuard like in Guido's linked example is following the PEP 647 spec. With this PEP's proposed change, type checkers might not work correctly on their codebase any more:

def print_it(x: int | str) -> None:
    if is_positive_int(x):
         print("it's positive!")
    else:
         print(f"it's a string: {len(x)}")

With PEP 647 semantics, type checkers would catch the mistake in the else block. With this PEP's semantics, they would not.

Now, you can make the argument that such code is contrived and unlikely to appear in practice, as @erictraut did in python/typing#996 (comment).

But the PEP needs to acknowledge that it does break compatibility, and discuss why that is OK and how we can mitigate the possible harm. (For example, could type checkers temporarily raise a warning on encountering a TypeGuard that will now behave differently?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think type checkers should be trying to check for all possible scenarios where this might happen. As in PEP 647, people can make invalid TypeGuards.

In Guido's example, pyright at least couldn't tell that something might happen. It doesn't understand the logic of the TypeGuard.

I think there is no mitigation other than people that write TypeGuards being smart enough to know when they're doing something that a typechecker can't possibly catch.


However, some use cases such as the one below can be simplified:

.. code-block:: python

class A():
pass
class B():
pass

def is_A(x: A | B) -> TypeGuard[A]:
return is_instance(x, A)


def is_B(x: A | B) -> TypeGuard[B]:
return is_instance(x, B)


def test(x: A | B):
if is_A(x):
# Do stuff assuming x is an 'A'
return
assert is_B(x)

# Do stuff assuming x is a 'B'
return


With this proposed change, the code above continues to work but could be
simplified by removing the assertion that x is of type B in the negative case:

.. code-block:: python

class A():
pass
class B():
pass

def is_A(x: A | B) -> TypeGuard[A]:
return is_instance(x, A)


def test(x: A | B):
if is_A(x):
# Do stuff assuming x is an 'A'
return

# Do stuff assuming x is a 'B'
return


How to Teach This
=================

The belief is that new users will assume this is how ``TypeGuard`` works in the
first place. Meaning this change should make ``TypeGuard`` easier to teach.


Reference Implementation
========================

A reference `implementation <https://github.com/microsoft/pyright/commit/9a5af798d726bd0612cebee7223676c39cf0b9b0>`__ of this idea exists in Pyright.


Rejected Ideas
==============

Originally a new ``StrictTypeGuard`` construct was proposed. A
``StrictTypeGuard`` would be similar to to a ``TypeGuard`` except it would
explicitly state that output type was a subtype of the input type. Type checkers
would validate that the output type was a subtype of the input type.

See this comment: `StrictTypeGuard proposal <https://github.com/python/typing/discussions/1013#discussioncomment-1966238>`__

This was rejected because for most cases it's not necessary. Most people assume
the negative case for ``TypeGuard`` anyway, so why not just change the
specification to match their assumptions?

Copyright
=========

This document is placed in the public domain or under the CC0-1.0-Universal
license, whichever is more permissive.