python / cpython

The Python programming language
https://www.python.org
Other
63.65k stars 30.49k forks source link

Add `NoneType.__or__` to allow for `None | None` in type aliases and annotations #107271

Open heni opened 1 year ago

heni commented 1 year ago

Bug report

According to PEP-484 None should be synonymous with NoneType for contexts referring to type hints. CPython currently has different semantics for the __or__ operator and None/NoneType.

>>> types.NoneType | types.NoneType
<class 'NoneType'>
>>> None | None
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for |: 'NoneType' and 'NoneType'

Since version 3.10, the __or__ operator has been massively used with type hints, and the following synthetic code does not work well.

def print_sub_one(val: int) -> None | None:
    if val <= 0:
        return None
    print(val - 1)
    return None

And mypy passes this code without errors, while CPython cannot handle loading such a method. Of course, this is synthetic and non-production code, but I can assume that code generators can generate similar examples.

This can be easily fixed by replacing the semantics of the __or__ operator with None, i.e. I suggest fixing that None | None = None

Your environment

I have Linux Ubuntu 23/04 but I believe that this kind of problem isn't system/OS specific and related to any python 3.10+

terryjreedy commented 1 year ago

HEAVILY EDITED: None | None is a pure python expression that has nothing to do with the typing module or typing contexts as presented (but see below). The | operator is only defined as instance method for (AFAIK) ints, sets, and fairly recently, dicts and types (the latter for optional typing).

terryjreedy commented 1 year ago

(EDITED): Continuing: PEP-484 specifically says

When used in a type hint, the expression None is considered equivalent to type(None).

According to Firefox's search, the symbol | does not appear in this PEP. So the author did not obviously intend (edit: as the time of writing) that the PEP be interpreted as requiring that .__or__ be added as a NoneType instance method. (I think doing so would be ugly and likely to generate 'bug' reports if noticed.) If it is, the None doc entry should perhaps mentions that it is only present for typing expressions.

terryjreedy commented 1 year ago

The following work now because type.__or__ accepts None and because type.__ror__ is invoked when needed.

def f() -> str | None: pass
def f() -> None | str: pass

I was thinking up to now that there is no need to ever write None | None. (True for that literal text.) However, I concocted this unlikely but possible example which arguably should not raise.

>>> m, n = None, None
>>> def f() -> m | n: pass
... 
Traceback (most recent call last):
  File "<pyshell#28>", line 1, in <module>
    def f() -> m | n: pass
TypeError: unsupported operand type(s) for |: 'NoneType' and 'NoneType'
heni commented 1 year ago

In fact, mypy forces me to use None in type expressions, even if I intentionally want to use the safer NoneType.

(issue-107271) heni@Eugene-XPS-13:~/projects/$ cat test4.py
from types import NoneType

def fn(a: int) -> NoneType:
    pass

(issue-107271) heni@Eugene-XPS-13:~/projects/$ mypy test4.py 
test4.py:3: error: NoneType should not be used as a type, please use None instead  [valid-type]
Found 1 error in 1 file (checked 1 source file)

So I think it is better to fully mimic the None behavior for pure types. Even in the strange semantics of None | None.

heni commented 1 year ago

And I have closer to production code that is affected by these semantics. It's fully accepted by mypy and raises exception in runtime.

import functools

def run_or(bypass_fn):
    def decorator(f):
        f_result = f.__annotations__["return"]
        bypass_result = bypass_fn.__annotations__["return"]

        @functools.wraps(f)
        def wrapper(a: int) -> f_result | bypass_result:
            try:
                return f(a)
            except:
                # it's better to log it
                pass
            return bypass_fn(a)

        return wrapper
    return decorator

def fn_default(a: int) -> None:
    print(f"fn2({a})")

@run_or(fn_default)
def fn_specific(a: int) -> None:
    assert a > 0, "argument should be positive"
    print(f"fn1({a})")

if __name__ == "__main__":
    fn_specific(1)
heni commented 1 year ago

Funny thing that GPT-4 found working way for fixing the latest code example. It looks ugly, but it's the only way to get it to work without changes in cPython 😢

--- test3.py.bak    2023-07-26 10:15:29.679322189 +0300
+++ test3.py    2023-07-26 10:15:45.542279656 +0300
@@ -6,7 +6,7 @@
         bypass_result = bypass_fn.__annotations__["return"]

         @functools.wraps(f)
-        def wrapper(a: int) -> f_result | bypass_result:
+        def wrapper(a: int) -> (f_result | bypass_result if f_result or bypass_result else None):
             try:
                 return f(a)
             except:
JelleZijlstra commented 1 year ago

The relevant PEP here is PEP-604, not PEP-484. I do think there's a reasonable argument for making None | None work, as other objects used in types generally support |, and there are conceivable cases where users would end up executing None | None.

AlexWaygood commented 1 year ago

I can see a good argument in both directions here:

  1. I can see how you would end up with a situation where you might be doing None | None in the context of a type alias or type annotation. I can see that it might end up being pretty confusing when that didn't work, since most other types support |.
  2. Part of the "point of None" is that it's a sentinel singleton which supports essentially no operations on it. Adding NoneType.__or__ would cut against that in a striking way.
AlexWaygood commented 1 year ago

I feel like this is on the line between a "bug" and a "feature request" in terms of whether things are working according to how the specification promises they'd work. As @JelleZijlstra says, the relevant PEP here is really PEP-604 rather than PEP-484. PEP-484 says some things about None versus NoneType, it's true, but the writers of PEP-484 didn't anticipate the later changes that would be made in PEP-604, so what PEP-484 has to say isn't really relevant here.


PEP-604 has two things to say. The first cuts against adding NoneType.__or__; the second argues in favour of it.

(1)

Inspired by Scala [2] and Pike [3], this proposal adds operator type.__or__().

(Note that PEP-604 only proposes adding type.__or__() in terms of changes to builtin types; it doesn't propose any other changes to builtin types.)

(2)

The existing typing.Union and | syntax should be equivalent.

[...]

Optional values should be equivalent to the new union syntax None | t == typing.Optional[t]


Regardless, if we made this change, I think it's highly unlikely we'd backport it, so I'm recategorising this as a feature request.

sobolevn commented 1 year ago

I think that it is better to use typing.Union in this case. It is safe for all possible types.

I don't think that adding __or__ to None is safe, it can backfire in logical operations. For example: