-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Add note about limited propagation of attrs #35456
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
Explicitly mention that `attrs` are not retained once a series is placed into a dataframe, related to issue #35425
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.
Thanks for this, but I don't think this is the ideal location.
Right now we have a small section in user_guide/basics.rst
. In the future I suspect we'll have a page dedicated to attrs, but for now I think adding this note to basics.rst
is best.
@RobertRosca is this still active? Could you address Tom's comment if so? |
Co-authored-by: Tom Augspurger <TomAugspurger@users.noreply.github.com>
Thanks for updating - could you also address the comment
please? |
Hey, sorry for the delay, forgot about the PR 😅 Had a look at the And since attrs is marked as experimental, and adding your own metadata isn't really 'basic', I don't think it should be mentioned much on the page about essential basic functionality. Personally I'd expect to find this kind of warning about the use of attrs on the doc page for |
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.
@RobertRosca is this still active?
Hey, I added the note but to a different location to what @TomAugspurger suggested, if Tom's okay with where it is then hopefully it can be merged 😃 |
I don't think that a paragraph in the reference docs is the right place. It
should either go in the docstring or the user guide.
That said, the wording will need to be clarified. Right now it reads like
DataFrame just doesn't propagate attrs in any operation.
It shouldn't mention propagation, instead it should focus on the DataFrame
constructor not extracting attrs from individual Series.
Things like Series.to_frame() do correctly propagate attrs, so the focus
really should be on the constructor.
…On Tue, Nov 3, 2020 at 2:54 AM Robert Rosca ***@***.***> wrote:
Hey, I added the note but to a different location to what @TomAugspurger
<https://github.com/TomAugspurger> suggested, if Tom's okay with where it
is then hopefully it can be merged 😃
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#35456 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKAOIQXSMGMZ7XGLJCXTKTSN7ALRANCNFSM4PLSR7PQ>
.
|
Closing for now. @RobertRosca ping us know whenever you'd like to pick up and address @TomAugspurger feedback - we'll reopen |
Explicitly mention that
attrs
are not retained once a series is placed into a dataframe, related to issue #35425