Skip to content

Add SupportsLessThan to typing and typing_extensions #9

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

Closed
hauntsaninja opened this issue Oct 13, 2020 · 12 comments
Closed

Add SupportsLessThan to typing and typing_extensions #9

hauntsaninja opened this issue Oct 13, 2020 · 12 comments

Comments

@hauntsaninja
Copy link
Collaborator

The latest version of mypy / typeshed uses a protocol for things passed to sorted / sort. As measured by mypy_primer, this was a fairly disruptive change. To correctly type some usage of sort / sorted, you need the protocol, and it's pretty hard for users to figure this out, eg: elastic/eland#283 (comment)
Adding the protocol to typing / typing_extensions would make this solution more discoverable and involve less end user protocol redefinition boilerplate when used.

@srittau
Copy link
Collaborator

srittau commented Oct 14, 2020

Sounds like a good idea. In fact, adding protocols for other dunder protocols makes sense, too. See for example the dunder protocols in builtins.

@JelleZijlstra
Copy link
Member

Adding it to typing(_extensions) is relatively painful though because it ties us to Python's release cycle. An alternative could be to release our _typeshed stub-only module as a PyPI package with a runtime component. On the plus side for adding these things to typing though, it's more discoverable for users than a new package.

@srittau
Copy link
Collaborator

srittau commented Oct 14, 2020

Oops, I was misreading this. I was talking about _typeshed, not typing(_extensions), although I'm +0 to add it to the latter as well.

@V1NAY8
Copy link

V1NAY8 commented Oct 14, 2020

The Example given above was this

Item = TypeVar("Item")
def try_sort(iterable: Iterable[Item]) -> Iterable[Item]:
    # Pulled from pandas.core.common since
    # it was deprecated and removed in 1.1
    listed = list(iterable)
    try:
        return sorted(listed)
    except TypeError:
        return listed
  • mypy says: Value of type variable "_LT" of "sorted" cannot be "Item"

So, I changed it to str and it worked 😮

def try_sort(iterable: Iterable[str]) -> Iterable[str]:
    # Pulled from pandas.core.common since
    # it was deprecated and removed in 1.1
    listed = list(iterable)
    try:
        return sorted(listed)
    except TypeError:
        return listed

I am trying to figure out what was the general solution to this issue?

@gvanrossum
Copy link
Member

Adding it to typing(_extensions) is relatively painful though because it ties us to Python's release cycle.

Note that typing_extensions is not tied to the CPython release cycle.

@hauntsaninja
Copy link
Collaborator Author

@V1NAY8

The general solution if we add this to typing(_extensions) is:

SupportsLessThanT = typing.TypeVar("SupportsLessThanT", bound=typing.SupportsLessThan)
def function_that_wraps_sort(iterable: SupportsLessThanT) -> SupportsLessThanT: ...

If this isn't added to typing(_extensions), you'll need to define the SupportsLessThan protocol yourself , which looks like this.

But now that I'm looking closely at your code, some more comments:

  • What mypy does now is try to ensure that arguments to sorted are comparable. The above "general" solution will try to ensure that arguments to try_sort are comparable. But that doesn't seem to be what you want... your try_sort expects to raise a TypeError sometimes, which is naturally something that mypy should complain about! So I would continue to use your Item TypeVar and just type: ignore the sorted line, as you would if you were doing: try: 1 + sometimes_int_sometimes_str() except: TypeError: ...
  • In your case, if you can change the type to Iterable[str] without mypy complaining, it might mean you don't actually need try_sort and can just use sorted everywhere.

@everyoneelse

It sounds like no one is opposed, but no one feels strongly either? In that case, maybe we hold off and see if people complain, rather than presumptively making a decision based on mypy_primer.
It's also not clear to me what, if any, the process here is.

@guilhermeleobas
Copy link

@hauntsaninja, I am facing the same problem with min:

torch/distributions/kl.py:112: error: Value of type variable "_LT" of "min" cannot be "_Match"  [type-var]
torch/distributions/kl.py:113: error: Value of type variable "_LT" of "min" cannot be "_Match"  [type-var]

The lines involved are the ones below
https://github.com/pytorch/pytorch/blob/146721f1dfe29786f5d25a9e2a2c2227dfa85f76/torch/distributions/kl.py#L107-L108

@hauntsaninja
Copy link
Collaborator Author

@guilhermeleobas Hmm, your false positive is a little tricky. The problem is that there isn't a good way to type the return type of functools.total_ordering (python/mypy#4610 / intersection types), so mypy don't see that _Match does have __lt__ and things are in fact okay.

Some possibilities:

  • If we were to add SupportsLessThan to typing(_extensions) / if you define it yourself, you can do cast(SupportsLessThan, Match(...)) for m in matches)
  • You can use the implementation details of functools.total_ordering to somewhat sketchily add __lt__: Callable = None # type: ignore to the _Match class.
  • You can type ignore

@guilhermeleobas
Copy link

Thanks @hauntsaninja, I will give it a try.

@brettcs
Copy link

brettcs commented Feb 26, 2021

It sounds like no one is opposed, but no one feels strongly either? In that case, maybe we hold off and see if people complain, rather than presumptively making a decision based on mypy_primer.

I ended up here after upgrading my code from mypy 0.770 to 0.810, and this was the biggest issue I ran into. I have a handful of sort key functions that I had previously annotated as returning Hashable. I understand why Hashable wasn't right, I'm glad that's an error now, but I was a little sad there wasn't an easy low-level type to use in the typing module that I could correctly replace it with.

For me probably the best value I get out of type checking, after catching obvious bugs, is the way it helps document my code. Even though my sort key functions return higher-level types and I could annotate them as such, I like the low-level type annotation as a signal to say "I only promise this return value is comparable against other return values; don't try to do anything else with it." For that reason, I would appreciate an easy way to import "the return type of a sort key function."

butuzov referenced this issue in butuzov/deadlinks Mar 9, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 9, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 9, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
butuzov referenced this issue in butuzov/deadlinks Mar 10, 2021
After a year of not working o deadlinks, there are some broken things
that needs to be fixed:

- ci: move to github actions from travis-ci
- mypy: issue regarding totalordering -
  https://github.com/python/typing/issues/760
- tests: some tests are broken
- adding: act to test github actions integration.
@benbariteau
Copy link

I also ran into this issue.

@JelleZijlstra
Copy link
Member

Closing this as our current policy is to add only types that are in typing. #6 proposes changing that.

In addition, typeshed no longer has a SupportsLessThan type.

@JelleZijlstra JelleZijlstra closed this as not planned Won't fix, can't repro, duplicate, stale May 22, 2023
hannes-ucsc added a commit to DataBiosphere/azul that referenced this issue Nov 6, 2023
Upstream issue (python/typing_extensions#9) was closed without fix. SupportLessThan was removed from _typeshed. I believe it was never marked stable so it was never safe to import from there anyways.

https://github.com/python/typeshed/tree/main/stdlib/_typeshed

Instead of defining the protocol conditionally (`if TYPE_CHECKING:`), we're just going to define it always.
hannes-ucsc added a commit to DataBiosphere/azul that referenced this issue Nov 6, 2023
Upstream issue (python/typing_extensions#9) was closed without fix. SupportLessThan was removed from _typeshed. I believe it was never marked stable so it was never safe to import from there anyways.

https://github.com/python/typeshed/tree/main/stdlib/_typeshed

This just removes SupportsLessThan altogether. I checked a few places where we sort by BundleFQID and got no warnings from PyCharm.
hannes-ucsc added a commit to DataBiosphere/azul that referenced this issue Nov 7, 2023
Upstream issue (python/typing_extensions#9) was closed without fix. SupportLessThan was removed from _typeshed. I believe it was never marked stable so it was never safe to import from there anyways.

https://github.com/python/typeshed/tree/main/stdlib/_typeshed

This just removes SupportsLessThan and the implicit ordering of BundleFQID altogether.
hannes-ucsc added a commit to DataBiosphere/azul that referenced this issue Nov 7, 2023
Upstream issue (python/typing_extensions#9) was closed without fix. SupportLessThan was removed from _typeshed. I believe it was never marked stable so it was never safe to import from there anyways.

https://github.com/python/typeshed/tree/main/stdlib/_typeshed

This just removes SupportsLessThan and the implicit ordering of BundleFQID altogether.
hannes-ucsc added a commit to DataBiosphere/azul that referenced this issue Nov 7, 2023
Upstream issue (python/typing_extensions#9) was closed without fix. SupportLessThan was removed from _typeshed. I believe it was never marked stable so it was never safe to import from there anyways.

https://github.com/python/typeshed/tree/main/stdlib/_typeshed
achave11-ucsc pushed a commit to DataBiosphere/azul that referenced this issue Nov 7, 2023
Upstream issue (python/typing_extensions#9) was closed without fix. SupportLessThan was removed from _typeshed. I believe it was never marked stable so it was never safe to import from there anyways.

https://github.com/python/typeshed/tree/main/stdlib/_typeshed
hannes-ucsc added a commit to DataBiosphere/azul that referenced this issue Nov 7, 2023
Upstream issue (python/typing_extensions#9) was closed without fix. SupportLessThan was removed from _typeshed. I believe it was never marked stable so it was never safe to import from there anyways.

https://github.com/python/typeshed/tree/main/stdlib/_typeshed
hannes-ucsc added a commit to DataBiosphere/azul that referenced this issue Nov 7, 2023
Upstream issue (python/typing_extensions#9) was closed without fix. SupportLessThan was removed from _typeshed. I believe it was never marked stable so it was never safe to import from there anyways.

https://github.com/python/typeshed/tree/main/stdlib/_typeshed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants