-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
REGR: fix return class in _constructor_from_mgr for simple subclasses #55764
REGR: fix return class in _constructor_from_mgr for simple subclasses #55764
Conversation
cc @jorisvandenbossche im fine with the change here bc it improves perf the base classes. no strong opinion on the actual behavior. |
Do you mean that the check |
I've addressed the comment, added a test for subclasses of Series, and added a release note. This does throw a depreciation warning: import pandas as pd
class SimpleDataFrameSubClass(pd.DataFrame):
"""A subclass of DataFrame that does not define a constructor."""
s = SimpleDataFrameSubClass(pd.DataFrame({"a": [1, 2, 3]}))
s.copy()
But the previous fix seems to have done this as well. It would make sense if this didn't warn, since the user code in my example above can't obviously do anything about this warning besides silence it. It may be out of scope for this regression though, since this PR does not introduce this warning. |
Yes, we still have to figure out how to deal with this deprecation warning in subclasses. But, that deprecation warning is only for pandas 2.2, and so won't yet be visible in a pandas 2.1.3. And I think my other comment might solve this warning for the simple case where the subclass doesn't override the constructor. |
Co-authored-by: Isaac Virshup <[email protected]>
Co-authored-by: Isaac Virshup <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming the tests pass, this looks good to me!
Thanks @ivirshup ! |
…from_mgr for simple subclasses
…uctor_from_mgr for simple subclasses) (#55892) Backport PR #55764: REGR: fix return class in _constructor_from_mgr for simple subclasses Co-authored-by: Isaac Virshup <[email protected]>
copy
works #55763 (Replace xxxx with the GitHub issue number)doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.This is a draft for fixing #55763. I'm not familiar with this part of the code, but this seems to fix the issue locally.
This is addressing changes brought in by #54922. Instead of checking the class of
._constructor
in_constructor_from_mgr
, this checks the class being called.We were relying on most operations on our subclass returning a pandas dataframe. AFAICT, the previous PR changed the behavior so that most operations would return the class of the instance even if
_constructor
was not set.Once I see that this doesn't break tests on CI, added tests will be like:
added to
test_subclass.py
.