Skip to content

Fix binary operations on attrs for Series and DataFrame #59636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 37 commits into from
Apr 3, 2025
Merged
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
38efbaa
Fix binary operators on attrs for Series and DataFrame
fbourgey Aug 27, 2024
15db834
Add tests for Series and DataFrame for attrs binary operations
fbourgey Aug 28, 2024
50118d7
Add getattr as other might not possess attrs as attribute
fbourgey Sep 5, 2024
953ef17
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Sep 5, 2024
bc4da65
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Sep 6, 2024
2607f9c
Fix some tests in pandas/tests/generic/test_finalize.py::test_binops
fbourgey Sep 11, 2024
b154203
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Sep 11, 2024
87abede
remove xfail tests related to attrs
fbourgey Sep 13, 2024
8559c1f
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Sep 13, 2024
742e3b1
Add all_binary_operators
fbourgey Sep 24, 2024
5178113
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Sep 24, 2024
5b71171
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Dec 1, 2024
8da62bd
Merge branch 'pandas-dev:main' into series-sum-attrs
fbourgey Dec 2, 2024
c2c87dd
use __finalize__ for attrs bin ops in base.py
fbourgey Dec 2, 2024
4da1786
use __finalize__ for attrs bin ops in frame.py
fbourgey Dec 2, 2024
6436926
use __finalize__ for attrs bin ops in series.py
fbourgey Dec 2, 2024
50396ad
Merge branch 'main' into series-sum-attrs
fbourgey Jan 29, 2025
76b0831
Merge branch 'pandas-dev:main' into series-sum-attrs
fbourgey Jan 30, 2025
ef3c0b5
ENH: Ensure attrs are copied from other in Series arithmetic operatio…
fbourgey Feb 1, 2025
e92e431
Merge branch 'main' into series-sum-attrs
fbourgey Feb 1, 2025
311b662
Merge branch 'main' into series-sum-attrs
fbourgey Feb 1, 2025
95cb745
Merge branch 'pandas-dev:main' into series-sum-attrs
fbourgey Feb 3, 2025
c162bba
Merge branch 'main' into series-sum-attrs
fbourgey Feb 3, 2025
bd46491
Merge branch 'pandas-dev:main' into series-sum-attrs
fbourgey Feb 9, 2025
bf898c7
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Mar 29, 2025
e9c3e91
REF: Refactor binary operation tests for DataFrame and Series attributes
fbourgey Mar 30, 2025
b72a049
REF: Enhance _construct_result method to accept 'other' parameter for…
fbourgey Mar 30, 2025
3abc22d
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Mar 30, 2025
1e6197c
REF: Simplify test_attrs_binary_operations by parameterizing left and…
fbourgey Apr 1, 2025
3a80cf9
Refactor DataFrame and Series methods to improve attribute handling a…
fbourgey Apr 1, 2025
83e908f
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Apr 1, 2025
e342a6e
Refactor DataFrame alignment logic to improve attribute consistency
fbourgey Apr 2, 2025
c251196
Refactor DataFrame and Series methods to enhance attribute finalizati…
fbourgey Apr 2, 2025
96ddd0d
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Apr 2, 2025
155a9d1
Clean and remove unnecessry checks
fbourgey Apr 3, 2025
45e2ad0
Fix: Prioritizing False if self.flags.allows_duplicate_labels or othe…
fbourgey Apr 3, 2025
cf67fcf
Merge remote-tracking branch 'upstream/main' into series-sum-attrs
fbourgey Apr 3, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -8088,8 +8088,7 @@ def _align_for_op(

Parameters
----------
left : DataFrame
right : Any
other : Any
axis : int
flex : bool or None, default False
Whether this is a flex op, in which case we reindex.
Expand All @@ -8101,8 +8100,6 @@ def _align_for_op(
left : DataFrame
right : Any
"""
if not getattr(self, "attrs", None) and getattr(other, "attrs", None):
self.__finalize__(other)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

left, right = self, other

Expand Down Expand Up @@ -8203,7 +8200,6 @@ def to_series(right):
"`left, right = left.align(right, axis=1)` "
"before operating."
)

left, right = left.align(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
left, right = left.align(
left, right = left.align(

right,
join="outer",
Expand All @@ -8212,6 +8208,9 @@ def to_series(right):
)
right = left._maybe_align_series_as_frame(right, axis)
Copy link
Contributor Author

@fbourgey fbourgey Apr 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this resets the attrs of right

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider that a bug. attrs should be preserved in this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I fix it in this PR or raise a different issue?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can fix it in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested something below


# Ensure attributes are consistent between the aligned and original objects
if right.attrs != getattr(other, "attrs", {}):
right.attrs = getattr(other, "attrs", {}).copy()
return left, right

def _maybe_align_series_as_frame(self, series: Series, axis: AxisInt):
Expand Down
Loading