Skip to content

GH-49826: [Python] Return NotImplemented from Scalar/Array arithmetic dunders for unsupported types#49845

Open
alex-anast wants to merge 3 commits intoapache:mainfrom
alex-anast:alex-anast/gh-49826-python-scalar-dunder-return-notimplemented
Open

GH-49826: [Python] Return NotImplemented from Scalar/Array arithmetic dunders for unsupported types#49845
alex-anast wants to merge 3 commits intoapache:mainfrom
alex-anast:alex-anast/gh-49826-python-scalar-dunder-return-notimplemented

Conversation

@alex-anast
Copy link
Copy Markdown

@alex-anast alex-anast commented Apr 22, 2026

Rationale for this change

In pyarrow 24.0.0, Scalar and Array gained arithmetic dunder methods (#32007) that unconditionally dispatch to pyarrow.compute.call_function. When the other operand is an unrecognized type, _pack_compute_args raises TypeError instead of returning NotImplemented. This prevents Python from falling back to the other operand's reflected methods (__radd__, __rsub__, etc.), breaking downstream libraries that relied on this protocol.

What changes are included in this PR?

  • Adds a _compute_binary_op helper in scalar.pxi that wraps call_function in a try/except TypeError and returns NotImplemented on failure. This follows the same pattern already used by Scalar.__eq__ and Array.__eq__.
  • Updates all binary arithmetic and bitwise dunders on both Scalar (scalar.pxi) and Array (array.pxi) to use this helper.
  • Adds parametrized tests in test_scalars.py and test_array.py verifying that reflected operators on custom types are correctly invoked.

Are these changes tested?

Yes. New parametrized tests (test_dunders_return_notimplemented_for_unknown_types) cover all 10 binary operators for both Scalar and Array. The bug was also manually reproduced against pyarrow 24.0.0 to confirm the tests exercise the right code path.

Are there any user-facing changes?

Yes. Scalar and Array arithmetic dunders now return NotImplemented instead of raising TypeError when the other operand is not a recognized Arrow/NumPy type. This restores the pre-24.0.0 behavior where Python would fall back to the other operand's reflected method.

AI-generated code disclosure

This PR was developed with assistance from an AI coding tool (Claude, Anthropic). All changes have been reviewed, understood, and verified.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #49826 has been automatically assigned in GitHub to PR creator.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #49826 has been automatically assigned in GitHub to PR creator.

@AlenkaF
Copy link
Copy Markdown
Member

AlenkaF commented Apr 23, 2026

Thank you for opening a PR.
There is already one PR open: #49833 so we will proceed with that one first. The existing comments apply for both though. Also, please be upfront about the AI use in your PRs, see #49833.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #49826 has been automatically assigned in GitHub to PR creator.

1 similar comment
@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #49826 has been automatically assigned in GitHub to PR creator.

@alex-anast
Copy link
Copy Markdown
Author

@AlenkaF apologies, hadn't seen the other PR. I think there is something sketchy going on there, though, if you look at the user's profile, with e.g. ~280 git commits in a day, it might be that these PRs are fully auto-generated.

This one covers Array and also has tests, though, as you mentioned. In the most recent commit, I have also tried to address the narrower-catch comment, here.

Let me know if I should keep this PR or not.

@AlenkaF
Copy link
Copy Markdown
Member

AlenkaF commented Apr 23, 2026

@AlenkaF apologies, hadn't seen the other PR.

All good, no harm done ;)

I think there is something sketchy going on there, though, if you look at the user's profile, with e.g. ~280 git commits in a day, it might be that these PRs are fully auto-generated.

Yeah, lots of sketchy things going on in these days :) Happy to see a normal response on this PR!

This one covers Array and also has tests, though, as you mentioned. In the most recent commit, I have also tried to address the narrower-catch comment, here.

This is great, thanks! Will have a look tomorrow.

Let me know if I should keep this PR or not.

Yes, please do keep it. If there is no response on the other PR I plan to proceed with yours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Python] Scalar arithmetic dunders raise TypeError instead of returning NotImplemented

2 participants