Repository: apytypes/apytypes (language: python)

Issue:
Comparison with long integers
After #307 the comparison is done at bit-level by converting the incoming double to an APyFloat (and in the case of APyFixed comparison, the APyFloat to an APyFixed). However, for long Python integers this will not work correctly. There should be a method that creates a "long enough" APyFixed and then uses that for comparison. (The creation method should not be exported...)


Discussion / hints:

Fix patch:
diff --git a/lib/apytypes/_apytypes.pyi b/lib/apytypes/_apytypes.pyi
index 0c8283846..1a3ed9ea7 100644
--- a/lib/apytypes/_apytypes.pyi
+++ b/lib/apytypes/_apytypes.pyi
@@ -125,15 +125,21 @@ class APyFixed:
     def __eq__(self, arg0: APyFixed) -> bool: ...
     @typing.overload
     def __eq__(self, arg0: float) -> bool: ...
+    @typing.overload
+    def __eq__(self, arg0: int) -> bool: ...
     def __float__(self) -> float: ...
     @typing.overload
     def __ge__(self, arg0: APyFixed) -> bool: ...
     @typing.overload
     def __ge__(self, arg0: float) -> bool: ...
     @typing.overload
+    def __ge__(self, arg0: int) -> bool: ...
+    @typing.overload
     def __gt__(self, arg0: APyFixed) -> bool: ...
     @typing.overload
     def __gt__(self, arg0: float) -> bool: ...
+    @typing.overload
+    def __gt__(self, arg0: int) -> bool: ...
     def __init__(
         self,
         bit_pattern: int,
@@ -145,12 +151,16 @@ class APyFixed:
     def __le__(self, arg0: APyFixed) -> bool: ...
     @typing.overload
     def __le__(self, arg0: float) -> bool: ...
+    @typing.overload
+    def __le__(self, arg0: int) -> bool: ...
     def __lshift__(self, shift_amnt: int) -> APyFixed: ...
     @typing.overload
     def __lt__(self, arg0: APyFixed) -> bool: ...
     @typing.overload
     def __lt__(self, arg0: float) -> bool: ...
     @typing.overload
+    def __lt__(self, arg0: int) -> bool: ...
+    @typing.overload
     def __mul__(self, arg0: APyFixed) -> APyFixed: ...
     @typing.overload
     def __mul__(self, arg0: int) -> APyFixed: ...
@@ -159,6 +169,8 @@ class APyFixed:
     @typing.overload
     def __ne__(self, arg0: float) -> bool: ...
     @typing.overload
+    def __ne__(self, arg0: int) -> bool: ...
+    @typing.overload
     def __neg__(self) -> APyFixed: ...
     @typing.overload
     def __neg__(self) -> APyFixed: ...
diff --git a/src/apyfixed.cc b/src/apyfixed.cc
index 5ec3ab98a..63211f455 100644
--- a/src/apyfixed.cc
+++ b/src/apyfixed.cc
@@ -47,7 +47,7 @@ APyFixed::APyFixed(const APyFixed& other)
 }
 
 APyFixed::APyFixed(
-    nb::int_ python_long_int_bit_pattern,
+    const nb::int_& python_long_int_bit_pattern,
     std::optional<int> int_bits,
     std::optional<int> frac_bits,
     std::optional<int> bits
@@ -83,21 +83,11 @@ APyFixed::APyFixed(int bits, int int_bits, _IT begin, _IT end)
     : APyFixed(bits, int_bits)
 {
     assert(std::distance(begin, end) > 0);
-    assert(std::distance(begin, end) <= ptrdiff_t(bits_to_limbs(bits)));
+    assert(std::distance(begin, end) == ptrdiff_t(bits_to_limbs(bits)));
 
     // Copy data into resulting vector
     std::copy(begin, end, _data.begin());
 
-    // Sign-extend if necessary
-    if (std::size_t(std::distance(begin, end)) < bits_to_limbs(bits)) {
-        mp_limb_t sign_limb = mp_limb_signed_t(*std::prev(end)) < 0 ? -1 : 0;
-        std::fill_n(
-            _data.begin() + std::distance(begin, end),
-            bits_to_limbs(bits) - std::distance(begin, end),
-            sign_limb
-        );
-    }
-
     // Two's-complements overflow bits outside of the range
     _overflow_twos_complement(_data.begin(), _data.end(), _bits, _int_bits);
 }
@@ -341,6 +331,60 @@ bool APyFixed::operator>=(const APyFixed& rhs) const
     return diff.is_negative() || diff.is_zero();
 }
 
+bool APyFixed::operator==(const nb::int_& rhs) const
+{
+    const std::vector<mp_limb_t> limb_vec = python_long_to_limb_vec(rhs);
+    APyFixed rhs_fixed(
+        _LIMB_SIZE_BITS * limb_vec.size(), _LIMB_SIZE_BITS * limb_vec.size(), limb_vec
+    );
+    return *this == rhs_fixed;
+}
+
+bool APyFixed::operator!=(const nb::int_& rhs) const
+{
+    const std::vector<mp_limb_t> limb_vec = python_long_to_limb_vec(rhs);
+    APyFixed rhs_fixed(
+        _LIMB_SIZE_BITS * limb_vec.size(), _LIMB_SIZE_BITS * limb_vec.size(), limb_vec
+    );
+    return *this != rhs_fixed;
+}
+
+bool APyFixed::operator<(const nb::int_& rhs) const
+{
+    const std::vector<mp_limb_t> limb_vec = python_long_to_limb_vec(rhs);
+    APyFixed rhs_fixed(
+        _LIMB_SIZE_BITS * limb_vec.size(), _LIMB_SIZE_BITS * limb_vec.size(), limb_vec
+    );
+    return *this < rhs_fixed;
+}
+
+bool APyFixed::operator<=(const nb::int_& rhs) const
+{
+    const std::vector<mp_limb_t> limb_vec = python_long_to_limb_vec(rhs);
+    APyFixed rhs_fixed(
+        _LIMB_SIZE_BITS * limb_vec.size(), _LIMB_SIZE_BITS * limb_vec.size(), limb_vec
+    );
+    return *this <= rhs_fixed;
+}
+
+bool APyFixed::operator>(const nb::int_& rhs) const
+{
+    const std::vector<mp_limb_t> limb_vec = python_long_to_limb_vec(rhs);
+    APyFixed rhs_fixed(
+        _LIMB_SIZE_BITS * limb_vec.size(), _LIMB_SIZE_BITS * limb_vec.size(), limb_vec
+    );
+    return *this > rhs_fixed;
+}
+
+bool APyFixed::operator>=(const nb::int_& rhs) const
+{
+    const std::vector<mp_limb_t> limb_vec = python_long_to_limb_vec(rhs);
+    APyFixed rhs_fixed(
+        _LIMB_SIZE_BITS * limb_vec.size(), _LIMB_SIZE_BITS * limb_vec.size(), limb_vec
+    );
+    return *this >= rhs_fixed;
+}
+
 bool APyFixed::operator==(const double rhs) const
 {
     APyFloat rhs_fp(
@@ -505,9 +549,15 @@ std::string APyFixed::to_string_dec() const
     return result;
 }
 
-std::string APyFixed::to_string_hex() const { throw NotImplementedException(); }
+std::string APyFixed::to_string_hex() const
+{
+    throw NotImplementedException("APyFixed::to_string_hex()");
+}
 
-std::string APyFixed::to_string_oct() const { throw NotImplementedException(); }
+std::string APyFixed::to_string_oct() const
+{
+    throw NotImplementedException("APyFixed::to_string_oct()");
+}
 
 std::string APyFixed::to_string(int base) const
 {
@@ -522,7 +572,10 @@ std::string APyFixed::to_string(int base) const
         return to_string_dec();
         break;
     default:
-        throw NotImplementedException();
+        throw nb::value_error(
+            fmt::format("APyFixed::to_string(base={}): base is not supported", base)
+                .c_str()
+        );
         break;
     }
 }
diff --git a/src/apyfixed.h b/src/apyfixed.h
index 1a7681e0c..b651e8dff 100644
--- a/src/apyfixed.h
+++ b/src/apyfixed.h
@@ -65,9 +65,12 @@ class APyFixed {
     //! No default constructed `APyFixed` objects
     APyFixed() = delete;
 
+    //! Construct a copy from `other`.
+    APyFixed(const APyFixed& other);
+
     //! Main Python-exposed `APyFixed` constructor
     explicit APyFixed(
-        nanobind::int_ python_long_int_bit_pattern,
+        const nanobind::int_& python_long_int_bit_pattern,
         std::optional<int> int_bits = std::nullopt,
         std::optional<int> frac_bits = std::nullopt,
         std::optional<int> bits = std::nullopt
@@ -78,9 +81,6 @@ class APyFixed {
      * ****************************************************************************** */
 
 public:
-    //! Construct a copy from `other`.
-    APyFixed(const APyFixed& other);
-
     //! Construct a zero-initialized number with `bits` and `int_bits`. Undefined
     //! behaviour if `bits < 1`.
     explicit APyFixed(int bits, int int_bits);
@@ -139,12 +139,21 @@ class APyFixed {
     bool operator<=(const APyFixed& rhs) const;
     bool operator>(const APyFixed& rhs) const;
     bool operator>=(const APyFixed& rhs) const;
+
+    bool operator==(const nanobind::int_& rhs) const;
+    bool operator!=(const nanobind::int_& rhs) const;
+    bool operator<(const nanobind::int_& rhs) const;
+    bool operator<=(const nanobind::int_& rhs) const;
+    bool operator>(const nanobind::int_& rhs) const;
+    bool operator>=(const nanobind::int_& rhs) const;
+
     bool operator==(const double rhs) const;
     bool operator!=(const double rhs) const;
     bool operator<=(const double rhs) const;
     bool operator<(const double rhs) const;
     bool operator>=(const double rhs) const;
     bool operator>(const double rhs) const;
+
     bool operator==(const float rhs) const;
     bool operator!=(const float rhs) const;
     bool operator<=(const float rhs) const;
diff --git a/src/apyfixed_wrapper.cc b/src/apyfixed_wrapper.cc
index e22e49e4a..01948c724 100644
--- a/src/apyfixed_wrapper.cc
+++ b/src/apyfixed_wrapper.cc
@@ -36,6 +36,13 @@ void bind_fixed(nb::module_& m)
         .def(nb::self > nb::self)
         .def(nb::self >= nb::self)
 
+        .def(nb::self == nb::int_())
+        .def(nb::self != nb::int_())
+        .def(nb::self < nb::int_())
+        .def(nb::self <= nb::int_())
+        .def(nb::self > nb::int_())
+        .def(nb::self >= nb::int_())
+
         .def(nb::self == float())
         .def(nb::self != float())
         .def(nb::self < float())
diff --git a/src/python_util.h b/src/python_util.h
index f34de4af6..56f7ded66 100644
--- a/src/python_util.h
+++ b/src/python_util.h
@@ -91,10 +91,11 @@
  * Python arbitrary long integer object to GMP limb vector
  */
 [[maybe_unused]] static APY_INLINE std::vector<mp_limb_t> python_long_to_limb_vec(
-    nanobind::int_ py_long_int, std::optional<std::size_t> n_exact_limbs = std::nullopt
+    const nanobind::int_& py_long_int,
+    std::optional<std::size_t> n_exact_limbs = std::nullopt
 )
 {
-    PyLongObject* py_long = (PyLongObject*)py_long_int.ptr();
+    const PyLongObject* py_long = (const PyLongObject*)py_long_int.ptr();
     long py_long_digits = PyLong_DigitCount(py_long);
     bool py_long_is_negative = PyLong_IsNegative(py_long);
 


Test patch:
diff --git a/lib/test/apyfixed/test_arithmetic.py b/lib/test/apyfixed/test_arithmetic.py
index 55b7c77e7..62872483d 100644
--- a/lib/test/apyfixed/test_arithmetic.py
+++ b/lib/test/apyfixed/test_arithmetic.py
@@ -1,6 +1,7 @@
 from apytypes import APyFixed
 
 import math
+import pytest
 
 
 def test_arithmetic_operations():
@@ -61,27 +62,28 @@ def test_binary_comparison():
     assert APyFixed.from_float(-1.0, 256, 128) >= APyFixed.from_float(-3.0, 256, 128)
 
 
-def test_float_comparison():
-    assert not APyFixed.from_float(0.0, 256, 128) == 1.0
-    assert APyFixed.from_float(0.0, 256, 128) != 1.0
-    assert APyFixed.from_float(0.0, 256, 128) < 1.0
-    assert APyFixed.from_float(0.0, 256, 128) <= 1.0
-    assert not APyFixed.from_float(0.0, 256, 128) > 1.0
-    assert not APyFixed.from_float(0.0, 256, 128) >= 1.0
-
-    assert APyFixed.from_float(1.0, 256, 128) == 1.0
-    assert not APyFixed.from_float(1.0, 256, 128) != 1.0
-    assert not APyFixed.from_float(1.0, 256, 128) < 1.0
-    assert APyFixed.from_float(1.0, 256, 128) <= 1.0
-    assert not APyFixed.from_float(1.0, 256, 128) > 1.0
-    assert APyFixed.from_float(1.0, 256, 128) >= 1.0
-
-    assert not APyFixed.from_float(-1.0, 256, 128) == -3.0
-    assert APyFixed.from_float(-1.0, 256, 128) != -3.0
-    assert not APyFixed.from_float(-1.0, 256, 128) < -3.0
-    assert not APyFixed.from_float(-1.0, 256, 128) <= -3.0
-    assert APyFixed.from_float(-1.0, 256, 128) > -3.0
-    assert APyFixed.from_float(-1.0, 256, 128) >= -3.0
+@pytest.mark.parametrize("type", [int, float])
+def test_float_and_int_comparison(type):
+    assert not APyFixed.from_float(0.0, 256, 128) == type(1.0)
+    assert APyFixed.from_float(0.0, 256, 128) != type(1.0)
+    assert APyFixed.from_float(0.0, 256, 128) < type(1.0)
+    assert APyFixed.from_float(0.0, 256, 128) <= type(1.0)
+    assert not APyFixed.from_float(0.0, 256, 128) > type(1.0)
+    assert not APyFixed.from_float(0.0, 256, 128) >= type(1.0)
+
+    assert APyFixed.from_float(1.0, 256, 128) == type(1.0)
+    assert not APyFixed.from_float(1.0, 256, 128) != type(1.0)
+    assert not APyFixed.from_float(1.0, 256, 128) < type(1.0)
+    assert APyFixed.from_float(1.0, 256, 128) <= type(1.0)
+    assert not APyFixed.from_float(1.0, 256, 128) > type(1.0)
+    assert APyFixed.from_float(1.0, 256, 128) >= type(1.0)
+
+    assert not APyFixed.from_float(-1.0, 256, 128) == type(-3.0)
+    assert APyFixed.from_float(-1.0, 256, 128) != type(-3.0)
+    assert not APyFixed.from_float(-1.0, 256, 128) < type(-3.0)
+    assert not APyFixed.from_float(-1.0, 256, 128) <= type(-3.0)
+    assert APyFixed.from_float(-1.0, 256, 128) > type(-3.0)
+    assert APyFixed.from_float(-1.0, 256, 128) >= type(-3.0)
 
 
 def test_narrow_add_sub():
diff --git a/lib/test/apyfixed/test_not_implemented.py b/lib/test/apyfixed/test_not_implemented.py
index d3d095434..290ca782c 100644
--- a/lib/test/apyfixed/test_not_implemented.py
+++ b/lib/test/apyfixed/test_not_implemented.py
@@ -3,10 +3,30 @@
 import pytest
 
 
-def test_not_implemented_yet():
+def test_from_str():
     with pytest.raises(ValueError, match="Not implemented"):
         APyFixed.from_str("0", bits=1, int_bits=0, base=8)
     with pytest.raises(ValueError, match="Not implemented"):
         APyFixed.from_str("0", bits=1, int_bits=0, base=16)
     with pytest.raises(ValueError, match="Not implemented"):
         APyFixed.from_str("0", bits=1, int_bits=0, base=-1)
+
+
+def test__str__():
+    with pytest.raises(ValueError, match=r"APyFixed::to_string_oct"):
+        APyFixed(0, 1, 0).__str__(base=8)
+    with pytest.raises(ValueError, match=r"APyFixed::to_string_hex"):
+        APyFixed(0, 1, 0).__str__(base=16)
+    with pytest.raises(ValueError, match=r"APyFixed::to_string"):
+        APyFixed(0, 1, 0).__str__(base=37)
+
+
+def test_missing_arithmetic():
+    with pytest.raises(ValueError, match=r"Not implemented"):
+        _ = APyFixed(0, 1, 0) + 4
+    with pytest.raises(ValueError, match=r"Not implemented"):
+        _ = APyFixed(0, 1, 0) - 4
+    with pytest.raises(ValueError, match=r"Not implemented"):
+        _ = APyFixed(0, 1, 0) * 4
+    with pytest.raises(ValueError, match=r"Not implemented"):
+        _ = APyFixed(0, 1, 0) / 4