-
Notifications
You must be signed in to change notification settings - Fork 5
✨: add CanArrayX protocols #32
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Ok This PR is doing too much. Let me pair it down to just a few Protocols and do the rest as a series of followups. |
Support unary minus operator. Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Support addition operator for array classes. Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Support subtraction operator for array classes Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Support multiplication operator for array classes Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Support true division operator for array classes
Support floor division operator for array classes
Support modulo operator for array classes
Support power operator for array classes
b47800d
to
96067a4
Compare
Support in-place addition operator for array classes
Support in-place subtraction operator for array classes
Support in-place multiplication operator for array classes
Support in-place true division operator for array classes
Support in-place floor division operator for array classes
Support in-place power operator for array classes
Support in-place modulo operator for array classes
Support right addition operator for array classes Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Support right subtraction operator for array classes
Support right multiplication operator for array classes
Support right division operator for array classes Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Support right floor division operator for array classes
Support right power operator for array classes
Support right modulo operator for array classes Signed-off-by: Nathaniel Starkman <nstarman@users.noreply.github.com>
Ping @NeilGirdhar, given related discussions. |
@@ -0,0 +1,502 @@ | |||
__all__ = ( |
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.
Not sure which should be public. All?
... | ||
|
||
|
||
class CanArrayAdd(Protocol): |
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.
I was thinking about parametrizing by dtype. Self, other, output. Bit of a mess. Maybe tackle parametrizing as a followup?
Should all the Protocols inherit from |
I don't know what Joren will say, but I would guess no and no? (I think you got it right in this PR?) Also, I'm guessing you're aware that |
My thought was for building stuff like class Positive(Protocol):
def __call__(self, array: CanArrayPos, /) -> CanArrayPos: ... is wrong. It should be something like class Positive(Protocol):
def __call__(self, array: HasArrayNamespace, /) -> HasArrayNamespace: ... But I think we want class Positive(Protocol):
def __call__(self, array: CanArrayPos, /) -> HasArrayNamespace: ... Which I think works best if it's class CanArrayPos(HasArrayNamespace, Protocol): ...
Yes. :). |
I see, you're kind of using it as a poor man's intersection?
Okay, is that because you're going to generate some documentation from these annotations? Or you find it less confusing? Also, are you going to add |
It's for 2 reasons: the array api does it in their docs and because I think the Python numerical tower is a mess and since ints and floats aren't subclasses of each other, it makes little sense for them to be interchangeable at the static type level. 😤😆
Worth discussing. The array api does not. |
The docs are that way to help beginners who might be confused. (At least that was the argument that was presented.) But you aren't expecting beginners to read your code, are you? And, you aren't using this repo to build docs? The downside of populating the unions unnecessarily is overcomplicated type errors. So from a user standpoint, I think this is worse. From a developer standpoint, it's a matter of taste. Personally, I think more succinct is easier to understand.
As much as you might like to turn back time and change the typing decisions that were made, the fact is that the static type I think I understand what you're doing and why. I spent years writing
array.__add__(other: int | float | complex | array, /) → array Have I misunderstood the documentation? |
Ah. We're building towards v2021 first. |
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.
It might be easier to use optype for this, as it already provides single-method generic protocols for each of the special dunders:
https://github.com/jorenham/optype/blob/master/optype/_core/_can.py
There's even documentation: https://github.com/jorenham/optype#binary-operations
And of course it's tested and thoroughly type-checked and stuff
No description provided.