Skip to content

Conversation

jbrockmendel
Copy link
Member

xref #62423, does not close.

This does not take a stand on whether to support list/tuple in the EA methods. An EA that currently supports it will continue to, likewise for arrays that don't support it.

Not taking that stand limits the amount of de-duplication/centralization we can do. Instead of all the exceptions being raised from unpack_zerodim_and_defer, helpers ops.get_op_exception_message and ops.get_shape_exception_message should at least help us get to consistent messages.

I did a pass to try to use these new helpers where relevant, but definitely didn't get them all. I also didn't implement _supports_(scalar|array)_op yet.

Before I go through the tests to update the exception messages, I'd like to get any bikeshedding out of the way about exactly what those messages should say.

@jbrockmendel
Copy link
Member Author

Brainstorming potential improvements to the messages:

  1. for reversed ops e.g __rsub__(x, y), we can make the message about sub(y, x) instead?
  2. In some places we have "do x instead" in the exception message. We could let the pattern accomodate that by having _supports_foo_op return tuple[bool, str | None] with the latter being an optional bit to add to the standard exception message.

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

Successfully merging this pull request may close these issues.

1 participant