-
Notifications
You must be signed in to change notification settings - Fork 27
Make a type-checking-only ObjectArray type
#308
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
Conversation
6c056f4 to
3e93e08
Compare
|
This would also help with some ugly bits in inducer/arraycontext#322, such as the overlap between array containers and numpy arrays that's causing overlap in |
3e93e08 to
17f05b3
Compare
17f05b3 to
dca83ae
Compare
|
Alright, since nobody jumped at the chance to provide a voice of reason, I'm plotting to proceed with this. |
alexfikl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few nitpick comments, but overall this looks reasonable to me!
79badc9 to
42fa8bd
Compare
|
I'm hitting a bit of a conundrum with this. On the one hand, I would like On the other hand, covariance prohibits arithmetic of the form because Options:
As you can tell, I'm leaning towards the first. I'd be happy to hear dissenting comments though. |
df94022 to
3fadb76
Compare
I'm not sure I understand all the ramifications of this, since I haven't been steeped into the current typing run, but this also sounds like the best option to me. It seems more useful for callers to be able to pass in cats when animals are expected in various functions than have the arithmetic methods be technically correct. When would pyright complain downstream for the arithmetic operations? I can see it complains here because the covariant
Very stupid (untested) thought, but would something like U = TypeVar("U", bound=T)
def __add__(self, other: U) -> Self: ...do anything useful? EDIT: A more believable hack: class SupportsArithmetic(Protocol):
def __add__(self, other: Any) -> Any: ...
class ObjectArray(Generic[ShapeT, T]):
def __add__(self, other: SupportsArithmetic) -> Self: ...? Nicely duplicates everything and not sure the |
Interesting thought, tried it out on the pyright playground. In short: no dice.
Nah, not loving the In the meantime, I've gathered a bit of experience now using this in |
3fadb76 to
c6f3256
Compare
@alexfikl @majosm What do you think of this?
Object arrays are super-duper in the way of the type checker understanding hierarchies of array containers. I can't for the life of me figure out how to teach numpy to take any old object as its dtype:
results in
In addition, shape-precise typing for numpy remains a "next year, for sure" type of thing. Where as in this reality, we can have all of that, today! 🪄
I somehow don't see much downside. Especially since it's type-checking only, it can be ripped out without damaging anybody's ability to run code. But maybe I'm too high on type checker juice here. Reality check please? 🙂
https://youtu.be/_oNgyUAEv0Q?si=4YId-5UMWjmQ4MbE&t=52