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

Iterable[T] not being dispatched #131

Open
ilan-gold opened this issue Feb 5, 2024 · 7 comments
Open

Iterable[T] not being dispatched #131

ilan-gold opened this issue Feb 5, 2024 · 7 comments

Comments

@ilan-gold
Copy link

ilan-gold commented Feb 5, 2024

From #27 (comment).

To reproduce:

from collections.abc import Iterable
import plum

@plum.dispatch
def f(x: Iterable[int]): print("int!")

@plum.dispatch
def f(x: Iterable[str]): print("str!")

f(['first', 'second']) # AmbiguousLookupError: `f(['first', 'second'])` is ambiguous.

Version 2.3.2 and python 3.10.13

@PhilipVinc
Copy link
Collaborator

PhilipVinc commented Feb 5, 2024

I had the doubt that this was caused by my recent refactoring and it is not. It's a bug that has existed for a while now.

I think it's because bear type does not really support parametric types...

@PhilipVinc
Copy link
Collaborator

This is the bug, cc @leycec , who probably knows already

In [1]: from beartype.door import is_bearable

In [2]: from collections.abc import Iterable

In [3]: is_bearable(['ci', 'ao'], Iterable[int])
Out[3]: True

@leycec
Copy link
Member

leycec commented Feb 6, 2024

Gah! Deep type-checking, huh? Yeah. @beartype mostly doesn't do that yet. I'm still nailing down the seemingly last remaining PEP 563 (i.e., from __future__ import annotations) bug for our upcoming @beartype 0.17.1 patch release. PEP 563: it is a special Hell.

Deep type-checking is what @beartype is all about in 2024, though. Let's optimistically choose to believe that @beartype will deeply type-check Iterable[...] type hints sometime this year. Obviously, that's not great. Later this year is better than nothing, but... if anyone has volunteer time to spare and would like to lend a helping PR hand over at @beartype, I would 😍 to gently guide you through the exact work needed to make this happen.

Until then, the pudgy bear cries tears of sadness for Plum.

@wesselb
Copy link
Member

wesselb commented Feb 11, 2024

@ilan-gold Although Iterable[T] isn't currently supported (though hopefully later this year 🚀!), you can get the desired behaviour by typing with list:

import plum

@plum.dispatch
def f(x: list[int]): print("int!")

@plum.dispatch
def f(x: list[str]): print("str!")

f(['first', 'second'])  # str!

Clearly this is not optimal, but it might provide a workable temporary solution. :)

@leycec
Copy link
Member

leycec commented Feb 12, 2024

Totally. Excellent workaround, @wesselb. For even more generality, consider collections.abc.Sequence[int] and collections.abc.MutableSequence[str] as well; @beartype deeply type-checks both similarly to tuple[str, ...] and list[int].

Indeed, this is awful. To help this happen sooner, I'm accelerating deep type-checking on the @beartype roadmap. My top priorities are now:

  • dict[...], collections.abc.Mapping[...], and collections.abc.MutableMapping[...]. Everybody wanted this since forever. Now, forever is just around the corner.
  • collections.abc.Iterable[...]. I didn't realize this was so hotly in demand. In hindsight... it makes sense. Everything of interest is iterable, after all.

I also didn't realize that Plum depended on deep type-checking for disambiguation. In hindsight... it makes sense. I like to think I would have prioritized all of this sooner if I'd known. Now, I can only say: "Woopsie." 🥴

@wesselb
Copy link
Member

wesselb commented Feb 12, 2024

@ilan-gold, @leycec, aha, of course Sequence is a much better workaround! In fact, that’s nearly as good as Iterable. :) Awesome.

@leycec Obviously there’s no need for any form of apology! Beartype is already providing an awesome feature set that’s more than sufficient. :) These are just cherries 🍒 on top of the already delicious cake 🎂.

@ilan-gold
Copy link
Author

Hi all! Yup, I did Sequence right after opening the issue, and it probably more faithfully captures my use-case :). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants