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

Support Ellipsis as a type per se #684

Closed
max-sixty opened this issue Oct 30, 2019 · 9 comments
Closed

Support Ellipsis as a type per se #684

max-sixty opened this issue Oct 30, 2019 · 9 comments
Labels
topic: feature Discussions about new features for Python's type annotations

Comments

@max-sixty
Copy link

Referenced from python/mypy#7818

We use an Ellipsis as an easy-to-understand, easy-to-import sentinel value to mean "everything else".

But it's not possible to use fully with type checking, since mypy doesn't exclude it from a type when it's been excluded with an if, as it does for None, or Enums per #240). I've moved the issue here since it's apparently a python typing issue rather than a mypy implementation.

I've included below a MCVE of the behavior below, and here's an example of how we use it, given @gvanrossum has already asked whether it's important to use an Ellipsis:

For example, to transpose an array, these are equivalent:

In [1]: import xarray as xr

In [2]: ds = xr.tutorial.scatter_example_dataset()

In [3]: ds
Out[3]:
<xarray.Dataset>
Dimensions:  (w: 4, x: 3, y: 11, z: 4)
Coordinates:
  * x        (x) int64 0 1 2
  * y        (y) float64 0.0 0.1 0.2 0.3 0.4 0.5 0.6 0.7 0.8 0.9 1.0
  * z        (z) int64 0 1 2 3
  * w        (w) <U5 'one' 'two' 'three' 'five'
Data variables:
    A        (x, y, z, w) float64 0.02074 0.04807 -0.1059 ... -0.1809 -0.04862
    B        (x, y, z, w) float64 0.0 0.0 0.0 0.0 ... 1.406 1.414 1.368 1.408

In [4]: ds.transpose('w','x','y','z')
Out[4]:
<xarray.Dataset>
Dimensions:  (w: 4, x: 3, y: 11, z: 4)
Coordinates:
  * x        (x) int64 0 1 2
  * y        (y) float64 0.0 0.1 0.2 0.3 0.4 0.5 0.6 0.7 0.8 0.9 1.0
  * z        (z) int64 0 1 2 3
  * w        (w) <U5 'one' 'two' 'three' 'five'
Data variables:
    A        (w, x, y, z) float64 0.02074 0.02074 0.02074 ... -0.03076 -0.04862
    B        (w, x, y, z) float64 0.0 0.002074 0.004147 ... 1.403 1.405 1.408


In [5]: ds.transpose('w',...) # use an Ellipsis to indicate 'all other dimensions'
Out[5]:
<xarray.Dataset>
Dimensions:  (w: 4, x: 3, y: 11, z: 4)
Coordinates:
  * x        (x) int64 0 1 2
  * y        (y) float64 0.0 0.1 0.2 0.3 0.4 0.5 0.6 0.7 0.8 0.9 1.0
  * z        (z) int64 0 1 2 3
  * w        (w) <U5 'one' 'two' 'three' 'five'
Data variables:
    A        (w, x, y, z) float64 0.02074 0.02074 0.02074 ... -0.03076 -0.04862
    B        (w, x, y, z) float64 0.0 0.002074 0.004147 ... 1.403 1.405 1.408

Code example with existing mypy:

import builtins
from typing import List, Union


def fun(x: Union[builtins.ellipsis, List], y: List):
    if x is not Ellipsis:
        y = x #  error: Incompatible types in assignment (expression has type "Union[ellipsis, List[Any]]", variable has type "List[Any]")

    return y

# another attempt:
def fun2(x: Union[Ellipsis, List], y: List): # error: Variable "builtins.Ellipsis" is not valid as a type
    if x is not Ellipsis:
        y = x

    return y

Thank you!

@gvanrossum
Copy link
Member

Sorry, you won't get much traction here either. The problem is that you're using Union[builtins.ellipsis, List], but that expression is not valid at runtime (I'm guessing you're getting away with it because of PEP 563).

I would be against adding the type ellipsis to Python's builtins module (also, if you were to request that, the proper channel would be bugs.python.org, not here). I could be convinced that it should be added to the stdlib types module though, but really, even for that I think your use case is not strong enough. Why can't you define your own singleton enum type for this purpose? It seems more "cute" than readable to reuse Ellipsis for this purpose.

FWIW I have no idea what to do with your MCVE. Does it just mean that xarray.Dataset.transpose uses this notation (clearly echoing its use in slices), and that therefore your own code should be allowed to use it for this purpose too? Are you a developer on the xarray project and is this blocking you from creating type annotations?

@max-sixty
Copy link
Author

FWIW I have no idea what to do with your MCVE. Does it just mean that xarray.Dataset.transpose uses this notation (clearly echoing its use in slices), and that therefore your own code should be allowed to use it for this purpose too? Are you a developer on the xarray project and is this blocking you from creating type annotations?

xarray has rolled out type annotations through the project (it's been great, and we fixed lots of problems we didn't know existed!). We have type: ignore around these sentinel values, given the issues above. I'm attempting to remove the ignore statements.

I included the transpose code in an attempt to demonstrate that we have a good use case for using ... in place of an Enum—i.e. because it's user-facing, as compared to the case you responded to previously. We can treat the type of the arguments to transpose as Union[str, Ellipsis]. I included the MCVE as case for mypy not discriminating on that Union.

That said, this is a convenience rather than a huge blocker. In particular, we can use the Enum pattern in the internal code.

@gvanrossum
Copy link
Member

Oh, so you are an xarray dev or contributor, and you're stuck because this is in xarray's public API (and it's naturally so because of the similarity with slices). That's a reasonable concern.

I came up with the following hack:

from typing import *

if TYPE_CHECKING:
    from enum import Enum
    class ellipsis(Enum):
        Ellipsis = "..."
    Ellipsis = ellipsis.Ellipsis
else:
    ellipsis = type(Ellipsis)

def f(a: Union[ellipsis, List], b: List) -> List:
    if a is Ellipsis:
        a = b
    return a

If that works for you now, there are two possible futures: either you could just stick with this, or you could lobby bugs.python.org to add something like

ellipsis = type(Ellipsis)

to typing.py in the stdlib. If that's successful we could add this to PEP 484 (or equivalent) and implement it in mypy and other type checkers. (Since it's not an enum it would still have to be special-cased. Alternatively we could lobby to make Ellipsis an enum, but that would probably require a PEP...)

@max-sixty
Copy link
Author

Great, thank for you the response @gvanrossum. That's a clever hack.

I'll put something into bugs.python.org

@ilevkivskyi
Copy link
Member

Just a small note, type checkers will need some more special-casing around ... when adding support for NumPy, I am fine with adding ellipsis to typing.py module (or maybe better to types.py), please open an issue on https://bugs.python.org.

@mpkocher
Copy link

I believe this might be the issue that was filed to address the ellipsis type.

https://bugs.python.org/issue41810

@gvanrossum
Copy link
Member

So is this supported in mypy yet? In any other checker? Is it in typeshed?

@jp-larose
Copy link

So is this supported in mypy yet? In any other checker? Is it in typeshed?

python/mypy#7818 was an issue raised to request support for ellipsis, but closed after it was linked to this issue.

@srittau srittau added the topic: feature Discussions about new features for Python's type annotations label Nov 4, 2021
@max-sixty
Copy link
Author

I think we can close this now — it's been implemented in the linked commit at https://bugs.python.org/issue41810.

mypy no longer needs ignores, so those were just removed in xarray: pydata/xarray#7343

Thank you to all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: feature Discussions about new features for Python's type annotations
Projects
None yet
Development

No branches or pull requests

6 participants