From b779c4b4b6ac1a64c202b669c7fc5bf5b452428f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Juaristi?= <127882282+juaristi22@users.noreply.github.com> Date: Wed, 16 Sep 2026 14:44:40 +0200 Subject: [PATCH] Fix issues from review: preserve operation weights --- .../weight-operation-metadata.fixed.md | 1 + microdf/microdataframe.py | 16 +- microdf/microseries.py | 90 +++----- microdf/tests/test_binary_weight_alignment.py | 195 ++++++++++++++++++ .../tests/test_dataframe_weight_storage.py | 47 +++++ 5 files changed, 273 insertions(+), 76 deletions(-) create mode 100644 changelog.d/weight-operation-metadata.fixed.md create mode 100644 microdf/tests/test_binary_weight_alignment.py diff --git a/changelog.d/weight-operation-metadata.fixed.md b/changelog.d/weight-operation-metadata.fixed.md new file mode 100644 index 00000000..2248aa70 --- /dev/null +++ b/changelog.d/weight-operation-metadata.fixed.md @@ -0,0 +1 @@ +Preserve label-aligned calling Series weights in binary operators and named arithmetic and comparison methods across pandas versions. Keep DataFrame reset-index weights independently mutable. diff --git a/microdf/microdataframe.py b/microdf/microdataframe.py index 30e0ff39..e756d44e 100644 --- a/microdf/microdataframe.py +++ b/microdf/microdataframe.py @@ -460,7 +460,7 @@ def reset_index( if inplace: # Snapshot weight *values* positionally — the index is about # to change and reset_index preserves row order. - weight_values = np.asarray(self.weights.values, dtype=float) + weight_values = np.array(self.weights, dtype=float, copy=True) super().reset_index( level=level, drop=drop, @@ -470,7 +470,7 @@ def reset_index( allow_duplicates=allow_duplicates, names=names, ) - self.weights = pd.Series(weight_values, index=self.index, dtype=float) + self.weights = weight_series(weight_values, self.index) self._link_all_weights() return None else: @@ -483,15 +483,9 @@ def reset_index( allow_duplicates=allow_duplicates, names=names, ) - out = MicroDataFrame(res, weights=self.weights.values) - # Ensure weights align to res.index (reset_index changes the - # index but preserves row order, so pass values positionally). - out.weights = pd.Series( - np.asarray(self.weights.values, dtype=float), - index=out.index, - dtype=float, - ) - return out + # Own a positional copy: reset_index changes labels but + # preserves row order. + return MicroDataFrame(res, weights=weight_series(self.weights, res.index)) def copy(self, deep: Optional[bool] = True) -> "MicroDataFrame": return super().copy(deep) diff --git a/microdf/microseries.py b/microdf/microseries.py index 2e2f6df3..61ce146b 100644 --- a/microdf/microseries.py +++ b/microdf/microseries.py @@ -120,6 +120,16 @@ def __finalize__(self, other, method=None, **kwargs): super().__finalize__(other, method=method, **kwargs) return finalize_weights(self, other, method, previous) + def _construct_result(self, *args, **kwargs): + # pandas has already aligned this Series before constructing a binary + # result. Retain its row weights, even when pandas 3 also finalizes + # metadata from the other operand. Delegate values and names to pandas. + result = super()._construct_result(*args, **kwargs) + if not isinstance(result, tuple): + result.weights = weight_series(self.weights, result.index) + # divmod constructs both tuple members through this same hook. + return result + def __setattr__(self, name, value): weights = self.__dict__.get("weights") if name == "index" else None super().__setattr__(name, value) @@ -890,95 +900,45 @@ def repeat(self, repeats, axis=None): def __getattr__(self, name: str) -> "MicroSeries": return MicroSeries(super().__getattr__(name), weights=self.weights) - # operators - - def __add__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__add__(other), weights=self.weights) - - def __sub__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__sub__(other), weights=self.weights) - - def __mul__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__mul__(other), weights=self.weights) - - def __floordiv__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__floordiv__(other), weights=self.weights) - - def __truediv__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__truediv__(other), weights=self.weights) - - def __mod__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__mod__(other), weights=self.weights) - - def __pow__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__pow__(other), weights=self.weights) - - def __xor__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__xor__(other), weights=self.weights) - - def __and__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__and__(other), weights=self.weights) - - def __or__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__or__(other), weights=self.weights) - - def __invert__(self) -> "MicroSeries": - return MicroSeries(super().__invert__(), weights=self.weights) - + # Explicit reverse overrides give this subclass priority when a plain + # pandas Series is on the left. _construct_result retains aligned weights. def __radd__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__radd__(other), weights=self.weights) + return super().__radd__(other) def __rsub__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__rsub__(other), weights=self.weights) + return super().__rsub__(other) def __rmul__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__rmul__(other), weights=self.weights) + return super().__rmul__(other) def __rfloordiv__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__rfloordiv__(other), weights=self.weights) + return super().__rfloordiv__(other) def __rtruediv__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__rtruediv__(other), weights=self.weights) + return super().__rtruediv__(other) def __rmod__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__rmod__(other), weights=self.weights) + return super().__rmod__(other) def __rpow__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__rpow__(other), weights=self.weights) + return super().__rpow__(other) def __rand__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__rand__(other), weights=self.weights) + return super().__rand__(other) def __ror__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__ror__(other), weights=self.weights) + return super().__ror__(other) def __rxor__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__rxor__(other), weights=self.weights) + return super().__rxor__(other) + + def __invert__(self) -> "MicroSeries": + return MicroSeries(super().__invert__(), weights=self.weights) def sqrt(self) -> "MicroSeries": sqrt_values = np.sqrt(self._values) return MicroSeries(sqrt_values, index=self.index, weights=self.weights) - # comparators - - def __lt__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__lt__(other), weights=self.weights) - - def __le__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__le__(other), weights=self.weights) - - def __eq__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__eq__(other), weights=self.weights) - - def __ne__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__ne__(other), weights=self.weights) - - def __ge__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__ge__(other), weights=self.weights) - - def __gt__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": - return MicroSeries(super().__gt__(other), weights=self.weights) - # assignment operators def __iadd__(self, other: Union[int, float, pd.Series]) -> "MicroSeries": diff --git a/microdf/tests/test_binary_weight_alignment.py b/microdf/tests/test_binary_weight_alignment.py new file mode 100644 index 00000000..15b16696 --- /dev/null +++ b/microdf/tests/test_binary_weight_alignment.py @@ -0,0 +1,195 @@ +"""Binary operations retain the calling Series' observation weights.""" + +import inspect + +import numpy as np +import pandas as pd +import pytest + +from microdf import MicroSeries + + +ARITHMETIC = ["add", "sub", "mul", "truediv", "floordiv", "mod", "pow"] +LOGICAL = ["and", "or", "xor"] +COMPARISONS = ["lt", "le", "eq", "ne", "ge", "gt"] + + +def assert_weighted_result(result, expected, source): + assert isinstance(result, MicroSeries) + pd.testing.assert_series_equal(pd.Series(result), expected) + expected_weights = ( + source.weights + if source.index.equals(expected.index) + else source.weights.reindex(expected.index) + ) + pd.testing.assert_series_equal(result.weights, expected_weights) + assert result.weights is not source.weights + assert result.sum() == expected.multiply(expected_weights).sum() + + +@pytest.mark.parametrize( + "method", + [f"__{prefix}{op}__" for op in ARITHMETIC + LOGICAL for prefix in ["", "r"]], +) +@pytest.mark.parametrize("weighted_other", [False, True]) +def test_binary_operators_align_weights_with_labels(method, weighted_other): + source = MicroSeries([10, 20], index=["b", "a"], weights=[1, 9], name="x") + other = pd.Series([2, 1], index=["a", "b"], name="x") + if weighted_other: + other = MicroSeries(other, weights=[9, 1]) + expected = getattr(pd.Series(source), method)(pd.Series(other)) + + result = getattr(source, method)(other) + + assert_weighted_result(result, expected, source) + # Addition is 22 * 9 + 11 * 1 = 209, rather than the positional 121. + if method in ["__add__", "__radd__"]: + assert result.sum() == 209 + result.weights.iloc[0] = 100 + np.testing.assert_array_equal(source.weights, [1, 9]) + if weighted_other: + np.testing.assert_array_equal(other.weights, [9, 1]) + source.weights.iloc[1] = 200 + assert result.weights.iloc[0] == 100 + + +@pytest.mark.parametrize( + "method", + ARITHMETIC + [f"r{op}" for op in ARITHMETIC] + COMPARISONS + ["div", "rdiv"], +) +@pytest.mark.parametrize("permuted", [False, True]) +def test_named_binary_methods_use_calling_series_weights(method, permuted): + source = MicroSeries([10, 20], index=["b", "a"], weights=[1, 9], name="x") + other = MicroSeries( + [2, 1], index=["a", "b"] if permuted else ["b", "a"], weights=[5, 7], name="x" + ) + expected = getattr(pd.Series(source), method)(pd.Series(other)) + + result = getattr(source, method)(other) + + assert_weighted_result(result, expected, source) + # Inherited public methods keep the installed pandas API signatures. + assert inspect.signature(getattr(MicroSeries, method)) == inspect.signature( + getattr(pd.Series, method) + ) + + +@pytest.mark.parametrize("method", [f"__{op}__" for op in COMPARISONS]) +def test_comparison_operators_keep_calling_series_weights_and_pandas_errors(method): + source = MicroSeries([10, 20], index=["b", "a"], weights=[1, 9]) + other = MicroSeries([20, 10], index=source.index, weights=[5, 7]) + expected = getattr(pd.Series(source), method)(pd.Series(other)) + assert_weighted_result(getattr(source, method)(other), expected, source) + + other.index = ["a", "b"] + with pytest.raises(ValueError) as pandas_error: + getattr(pd.Series(source), method)(pd.Series(other)) + with pytest.raises(ValueError) as microdf_error: + getattr(source, method)(other) + assert str(microdf_error.value) == str(pandas_error.value) + + +@pytest.mark.parametrize("method", ["__add__", "__rsub__", "add", "rsub", "lt"]) +@pytest.mark.parametrize("operand", [3, [2, 1], np.array([2, 1])]) +def test_scalar_and_array_binary_operands_keep_weights(method, operand): + source = MicroSeries([10, 20], index=["b", "a"], weights=[1, 9]) + expected = getattr(pd.Series(source), method)(operand) + assert_weighted_result(getattr(source, method)(operand), expected, source) + + +@pytest.mark.parametrize("method", ["__add__", "__rsub__", "add", "rsub", "lt"]) +def test_matching_duplicate_indexes_keep_positional_weights(method): + source = MicroSeries([10, 20, 30], index=["a", "a", "b"], weights=[1, 9, 3]) + other = MicroSeries([2, 1, 4], index=source.index, weights=[5, 7, 11]) + expected = getattr(pd.Series(source), method)(pd.Series(other)) + assert_weighted_result(getattr(source, method)(other), expected, source) + + +@pytest.mark.parametrize("method", ["__divmod__", "__rdivmod__", "divmod", "rdivmod"]) +def test_divmod_results_keep_calling_series_weights(method): + source = MicroSeries([10, 20], index=["b", "a"], weights=[1, 9]) + other = MicroSeries([3, 4], index=["a", "b"], weights=[5, 7]) + expected = getattr(pd.Series(source), method)(pd.Series(other)) + result = getattr(source, method)(other) + assert isinstance(result, tuple) + for actual, plain in zip(result, expected): + assert_weighted_result(actual, plain, source) + + +@pytest.mark.parametrize("method", ["__add__", "__rsub__", "add", "rsub", "lt"]) +@pytest.mark.parametrize( + "left_index,right_index", + [ + (["b", "a"], ["a", "c"]), + (["b", "a", "b"], ["a", "b", "b"]), + ], + ids=["new-rows", "ambiguous-duplicates"], +) +def test_binary_operations_reject_unknown_row_weights(method, left_index, right_index): + source = MicroSeries( + range(len(left_index)), index=left_index, weights=range(1, len(left_index) + 1) + ) + other = MicroSeries( + range(len(right_index)), + index=right_index, + weights=range(4, len(right_index) + 4), + ) + with pytest.raises(ValueError, match="weights"): + getattr(source, method)(other) + + +@pytest.mark.parametrize("method", ["add", "rsub", "lt"]) +def test_named_binary_arguments_preserve_pandas_values_and_errors(method): + index = pd.MultiIndex.from_tuples([("b", 2), ("a", 1)], names=["group", "row"]) + source = MicroSeries([np.nan, 20], index=index, weights=[1, 9]) + other = pd.Series([2, 1], index=pd.Index(["a", "b"], name="group")) + kwargs = {"level": "group", "fill_value": 0, "axis": "index"} + expected = getattr(pd.Series(source), method)(other, **kwargs) + assert_weighted_result(getattr(source, method)(other, **kwargs), expected, source) + for args, options in [ + ((other,), {"axis": 1}), + (([1],), {}), + ((other,), {"unknown": True}), + ]: + with pytest.raises((TypeError, ValueError)) as pandas_error: + getattr(pd.Series(source), method)(*args, **options) + with pytest.raises(type(pandas_error.value)) as microdf_error: + getattr(source, method)(*args, **options) + # pandas identifies the concrete subclass in invalid-axis messages. + expected_error = str(pandas_error.value).replace( + "object type Series", "object type MicroSeries" + ) + assert str(microdf_error.value) == expected_error + + +@pytest.mark.parametrize( + "operation", + [ + lambda plain, weighted: plain + weighted, + lambda plain, weighted: plain - weighted, + lambda plain, weighted: plain * weighted, + lambda plain, weighted: plain / weighted, + lambda plain, weighted: plain // weighted, + lambda plain, weighted: plain % weighted, + lambda plain, weighted: plain**weighted, + lambda plain, weighted: plain & weighted, + lambda plain, weighted: plain | weighted, + lambda plain, weighted: plain ^ weighted, + ], + ids=ARITHMETIC + LOGICAL, +) +@pytest.mark.parametrize("indexes", ["matching", "permuted", "duplicates"]) +def test_plain_series_left_expressions_preserve_weighted_dispatch(operation, indexes): + index = ["a", "a"] if indexes == "duplicates" else ["b", "a"] + source = MicroSeries([10, 20], index=index, weights=[1, 9], name="x") + other_index = ["a", "b"] if indexes == "permuted" else index + other = pd.Series([2, 1], index=other_index, name="x") + expected = operation(other, pd.Series(source)) + + result = operation(other, source) + + assert_weighted_result(result, expected, source) + result.weights.iloc[0] = 100 + np.testing.assert_array_equal(source.weights, [1, 9]) + source.weights.iloc[1] = 200 + assert result.weights.iloc[0] == 100 diff --git a/microdf/tests/test_dataframe_weight_storage.py b/microdf/tests/test_dataframe_weight_storage.py index fc766096..a0ece5f7 100644 --- a/microdf/tests/test_dataframe_weight_storage.py +++ b/microdf/tests/test_dataframe_weight_storage.py @@ -59,3 +59,50 @@ def test_stored_weight_edits_do_not_change_weight_column(dtype): np.testing.assert_array_equal(df["w"], [1, 2]) assert df.sum()["x"] == 10 * 100 + 20 * 2 + + +@pytest.mark.parametrize("drop", [False, True]) +@pytest.mark.parametrize("inplace", [False, True]) +@pytest.mark.parametrize( + "index,level", + [ + (pd.Index(["b", "a", "a"], name="row"), None), + ( + pd.MultiIndex.from_tuples( + [("b", 2), ("a", 1), ("a", 1)], names=["group", "row"] + ), + None, + ), + ( + pd.MultiIndex.from_tuples( + [("b", 2), ("a", 1), ("a", 1)], names=["group", "row"] + ), + "group", + ), + ], +) +def test_reset_index_owns_independently_mutable_weights(index, level, drop, inplace): + source = mdf.MicroDataFrame({"x": [10, 20, 30]}, index=index, weights=[1, 9, 3]) + original_weights = source.weights + expected = pd.DataFrame(source).reset_index(level=level, drop=drop) + + result = source.reset_index(level=level, drop=drop, inplace=inplace) + + if inplace: + assert result is None + result = source + assert isinstance(result, mdf.MicroDataFrame) + pd.testing.assert_frame_equal(pd.DataFrame(result), expected) + pd.testing.assert_series_equal( + result.weights, pd.Series([1.0, 9.0, 3.0], index=expected.index) + ) + assert result.x.sum() == 10 * 1 + 20 * 9 + 30 * 3 + assert result.weights is not original_weights + result.weights.iloc[0] = 100 + np.testing.assert_array_equal(original_weights, [1, 9, 3]) + assert result.x.sum() == 10 * 100 + 20 * 9 + 30 * 3 + if not inplace: + assert source.x.sum() == 10 * 1 + 20 * 9 + 30 * 3 + original_weights.iloc[1] = 200 + np.testing.assert_array_equal(result.weights, [100, 9, 3]) + assert result.x.sum() == 10 * 100 + 20 * 9 + 30 * 3