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

MAINT: Remove unsafe unions and ABCs from return-annotations #18915

Merged
merged 5 commits into from May 5, 2021

Conversation

BvB93
Copy link
Member

@BvB93 BvB93 commented May 5, 2021

Backport of #18885.

Per the title, this PR removes unsafe unions and abstract baseclasses from the return annotations,
e.g. functions that currently have one of the following patterns.

from __future__ import annotations
from typing import Any, Sequence, Any as A, Any as B

def func1(*args: Any, **kwargs: Any) -> A | B:
    pass

def func2(*args: Any, **kwargs: Any) -> Sequence[Any]:
    pass

The Problem

The problem with returning a Union (or, almost equivalently, an abstract-ish baseclass such as generic)
is that any and all operations performed on a union must be compatible with all of its members.
For example, operations that are exclusive to either np.float64 and np.ndarray are thus not allowed
to be executed by np.float64 | np.ndarray, unless the union is narrowed down via an explicit isinstance
check a priori.

While returning a union thus adds some a form of simplicity to the annotations (as we don't have to
distinguish between 0D and ND array-likes), the Union type is simply not suited for what we're trying
to describe here (xref python/mypy#1693). This would be a different story
for a hypothetical UnsafeUnion type, one where operations must be compatible with any member
of the union. Such type does not exist though.

from __future__ import annotations
from typing import Any
import numpy as np

array: np.ndarray[Any, np.dtype[np.float64]]
out = np.isneginf(array)

if TYPE_CHECKING:
    # note: Revealed type is 'Union[numpy.bool_, numpy.ndarray[Any, numpy.dtype[numpy.bool_]]]'
    reveal_type(out)

for i in out:
    # error: Item "bool_" of "Union[bool_, ndarray[Any, dtype[bool_]]]" has no attribute "__iter__" (not iterable)
    print(i)

# Only now will things
if isinstance(out, np.ndarray):
    for i in out:
        print(i)
else:
    print(out)

The Solution

The solutions implemented herein fall in either one of the following two categories:

  • Simply set the return type to Any. This is a simple, but non-ideal solution, as it removes any type
    safety for objects returned by aforementioned functions. Nevertheless, there is a group of functions
    that still needs to-be updated for dtype-support anyway (e.g. those in np.core.fromnumeric),
    so setting their return to Any is by no means the worst thing that can happen.
    A second group of functions is those where the use of Any is simply a necasity, as the output type is.
    For example, determined by the value of string literals (see np.einsum). As a silver lining: the einsum
    problem in particular does seem like it can be resolved with relative ease via a future mypy plugin.
  • Add additional an additional overload for 0D array-likes. This is the more thorough and permanent fix
    for the unsafe-union issue; it has been applied to the more recently annotated modules such as
    np.lib.ufunclike.
    There is however one important caveat here: as we currently lack shape-support (Typing support for shapes #16544) it is
    currently impossible to distinguish between 0D and ND ndarrays, and thus we are unable to describe
    the 0D-to-scalar casting that numpy aggressively performs on 0D arrays. While this should change
    once PEP 646 is live, in the mean time users will have to settle for a typing.cast call or a # type: ignore
    comment if it is known in advance that 0D-to-scalar cast will be performed.

@BvB93 BvB93 added this to the 1.20.3 release milestone May 5, 2021
@charris charris merged commit c40cef5 into numpy:maintenance/1.20.x May 5, 2021
@charris
Copy link
Member

charris commented May 5, 2021

Thanks Bas.

@BvB93 BvB93 deleted the unsafe branch May 5, 2021 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants