array-api icon indicating copy to clipboard operation
array-api copied to clipboard

Improve ellipsis type

Open aaronmondal opened this issue 2 years ago • 6 comments

The current ellipsis = TypeVar('ellipsis') doesn't behave well with typechecking. It's also unclear in the Markdown docs what ellipsis is supposed to be. It's kind of clear from the context, but not compatible with "real" python types:

from types import EllipsisType
from typing import Protocol, TypeVar

ellipsis = TypeVar('ellipsis')  # This is the current implementation.

class BadProto(Protocol):
    def f(self, ell: ellipsis) -> None: ...  # Error: TypeVar "ellipsis" appears onlly once in generic function signature


class GoodProto(Protocol):
    def f(self, ell: EllipsisType) -> None: ...  # Ok, no error raised.


def some_bad_operation(x: BadProto) -> None:
    x.f('hello')  # Problematic: doesn't raise a type error.

def some_good_operation(x: GoodProto) -> None:
    x.f('hello')  # Good, raises:
    # [Pyright] Argument of type "Literal['hello']" cannot be assigned to parameter "ell" of type "EllipsisType" in function "f"
    #   "Literal['hello']" is incompatible with "EllipsisType"

The downside of EllipsisType is that it's only available in Python 3.10 onwards. However, we have the following:

assert EllipsisType == type(...)
assert EllipsisType == type(Ellipsis)
assert EllipsisType != ...
assert EllipsisType != Ellipsis
assert ellipsis != Ellipsis  # with ellipsis = TypeVar('ellipsis')

It seems to me like it would make sense to change the current ellipsis = TypeVar('ellipsis') to one of the variants that works. I think it would also be a clarification in the documentation to change ellipsis to EllipsisType and maybe have a note somewhere that those using Python < 3.10 can use the equivalent type(...) or type(Ellipsis).

Potentially related to

  • #589
  • #229

aaronmondal avatar Sep 15 '23 15:09 aaronmondal

Thanks @aaronmondal, that sounds like a useful improvement to me. EllipsisType seems clear enough, it seems unambiguous either way that it means the ... syntax (or the Ellipsis builtin works too, but I've never seen it used in real-world case).

rgommers avatar Sep 15 '23 16:09 rgommers

and maybe have a note somewhere that those using Python < 3.10

A code comment should be fine I think - it doesn't seem desirable to include something like that in the html docs for the standard.

rgommers avatar Sep 15 '23 16:09 rgommers

@rgommers Cool, I'll send a PR soon :blush:

aaronmondal avatar Sep 15 '23 18:09 aaronmondal

Prior to 3.10 you can always use builtins.ellipsis. It's a bit of a semi-private type-check only type (xref https://github.com/python/typeshed/issues/7580), but it's fully functional otherwise and correctly type checks.

EDIT: types.EllipsisType is even defined as an builtins.ellipsis alias in typeshed, so it's not going anywhere any time soon.

BvB93 avatar Sep 21 '23 15:09 BvB93

it would also be a clarification in the documentation to change ellipsis to EllipsisType

This tripped me up - I expected just to be able to copy-paste the signature in the API docs into my implementation.

It took me down a rabbit hole before I eventually found that EllipsisType exists (in python 3.10+). 👍 for changing this in the docs - if anyone has to be sent down a rabbit hole it should be users of python <3.10.

I'm happy to submit a PR to fix this.

TomNicholas avatar Jul 10 '24 17:07 TomNicholas

Agree we should make the type annotations copy-pastable when that is possible.

asmeurer avatar Jul 10 '24 18:07 asmeurer