PyO3 / PyO3/pyo3

reversible arithmetic operators don't respect subclassing if only one arm implemented

Open
#6,024 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
16.2k
Forks
1k
Avg merge
2d 6h
Merged PRs (30d)
66

Description

If a #[pyclass] defines only one of __add__ or __radd__, any base class implementation for the unimplemented operation is ignored.

The following patch added to test_arithmetics creates a failing test:

diff --git a/tests/test_arithmetics.rs b/tests/test_arithmetics.rs
index bf3f99c34..44f811194 100644
--- a/tests/test_arithmetics.rs
+++ b/tests/test_arithmetics.rs
@@ -363,7 +363,7 @@ fn rhs_arithmetic() {
     });
 }
 
-#[pyclass]
+#[pyclass(subclass)]
 struct LhsAndRhs {}
 
 impl std::fmt::Debug for LhsAndRhs {
@@ -496,6 +496,27 @@ fn lhs_fellback_to_rhs() {
     });
 }
 
+#[pyclass(extends = LhsAndRhs)]
+struct OverrideRhs {}
+
+#[pymethods]
+impl OverrideRhs {
+    fn __radd__(&self, other: &Bound<'_, PyAny>) -> String {
+        format!("{other:?} + OverrideRhs")
+    }
+}
+
+#[test]
+fn override_rhs() {
+    Python::attach(|py| {
+        let c = Py::new(py, LhsAndRhs {}).unwrap();
+        let sub = Py::new(py, (OverrideRhs {}, LhsAndRhs {})).unwrap();
+        py_run!(py, c sub, "assert 1 + sub == '1 + OverrideRhs'");
+        // `__add__` not overridden, should use `LhsAndRhs.__add__`
+        py_run!(py, c sub, "assert sub + 1 == 'LA + 1'");
+    });
+}
+
 #[pyclass]
 struct RichComparisons {}

In the pure-Python equivalent situation the subclassing is respected:

class Lhs:
    def __add__(self, other):
        return f'LHS + {other}'

class Rhs(Lhs):
    def __radd(self, other):
        return f'{other} + Rhs'

1 + Rhs()
#> '1 + Rhs'
Rhs() + 1
#> 'LHS + 1'

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in tests/test_arithmetics.rs with the proposed override_rhs regression test and run the arithmetic tests to reproduce the failure. Trace the #[pyclass] arithmetic dispatch for inherited add and radd methods. Done means the subclass uses its implemented radd, while its unimplemented add still resolves to LhsAndRhs.add.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, rust
Domain
backend, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
62/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.