-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
API: define _constructor_expanddim for subclassing Series and DataFrame #9802
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
Conversation
as an aside can u put a small doc that shows how to subclass DataFrame now that it is reasonably supportered - maybe put it in the FAQ |
Sure, will do. By the way, is there any difference between FAQ and Gotcha? Otherwise, it may better to merge them to be more searchable. |
@jorisvandenbossche what about merging faq and gotchas? I would say yes |
Added subclassing documentation. Pls review. |
878199d
to
1444fac
Compare
@sinhrks This documentation on subclassing is great I don't have much feedback on the particulars because I have not done much subclassing of dataframe myself. However, we should clarify two things:
|
Subclassing pandas Data Structures | ||
---------------------------------- | ||
|
||
This section describes how to subclass ``pandas`` data structures to meet more specific needs. There are 2 points to be cared: |
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.
I would add a note here, something like:
In general you should prefer to use composition rather than inheritance with pandas data structures., see here.
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.
There are 2 points which need attention:
Nice docs! I agree with the questions raised by @shoyer, although I cannot say much about it myself as I never did some subclassing of dataframes. But one remark: I don't think this belongs in a FAQ, but that we should start with some 'advanced' section in the docs (together with internals or so) |
@sinhrks pls add a mention to try using |
@sinhrks ok as @jorisvandenbossche just mentioned, let's move this to the internals.rst section. |
Fixed @jreback 's points, and moved it to internals.rst. And preparing a composition example in separate PR. |
~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
To let original data structures have additional properties, you should let ``pandas`` knows what properties are added. ``pandas`` maps unknown properties to data names overriding ``__getattribute__``. Defining original properties can be done either ways: | ||
|
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.
in one of 2 ways:
@sinhrks lgtm, some minor comments. I think you can add a release note in enhancements. Trying not to 'advertise' this too much :) |
6600e39
to
d0099d1
Compare
@jreback Thanks, fixed your points, and added a release note. |
@sinhrks awesome. I sent you an invite to change the pandas push permissions. As soon as you accept, pls make this your first PR to merged! |
Thanks, let me merge this once after rebased. |
API: define _constructor_expanddim for subclassing Series and DataFrame
@sinhrks awesome! |
Did we ever resolve the question of whether the subclassing API is officially guaranteed or not? I suppose the reality is that people are going to subclass whether we tell them it's OK or not. |
@shoyer well enough warnings to not encourage it it's pretty doable nowadays (though imho composition is much more useful) |
Closes #9762. Does this needs release note?