-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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
API/DEPR: Rename DataFrame.applymap to DataFrame.map #52364
API/DEPR: Rename DataFrame.applymap to DataFrame.map #52364
Conversation
1b1ad9d
to
70e50f3
Compare
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.
lgtm, cc @pandas-dev/pandas-core for any thoughts
In the file: pandas/doc/source/user_guide/style.ipynb there is a linked reference to DataFrame.applymap in a cell, under "Methods to add styles". It is probably best to also relink this, now, to avoid it being forgotten and breaking later. |
Ok, I’ve done that @attack68. |
@@ -352,7 +352,7 @@ | |||
"\n", | |||
"- Using [.set_table_styles()][table] to control broader areas of the table with specified internal CSS. Although table styles allow the flexibility to add CSS selectors and properties controlling all individual parts of the table, they are unwieldy for individual cell specifications. Also, note that table styles cannot be exported to Excel. \n", | |||
"- Using [.set_td_classes()][td_class] to directly link either external CSS classes to your data cells or link the internal CSS classes created by [.set_table_styles()][table]. See [here](#Setting-Classes-and-Linking-to-External-CSS). These cannot be used on column header rows or indexes, and also won't export to Excel. \n", | |||
"- Using the [.apply()][apply] and [.applymap()][applymap] functions to add direct internal CSS to specific data cells. See [here](#Styler-Functions). As of v1.4.0 there are also methods that work directly on column header rows or indexes; [.apply_index()][applyindex] and [.applymap_index()][applymapindex]. Note that only these methods add styles that will export to Excel. These methods work in a similar way to [DataFrame.apply()][dfapply] and [DataFrame.applymap()][dfapplymap].\n", | |||
"- Using the [.apply()][apply] and [.applymap()][applymap] functions to add direct internal CSS to specific data cells. See [here](#Styler-Functions). As of v1.4.0 there are also methods that work directly on column header rows or indexes; [.apply_index()][applyindex] and [.applymap_index()][applymapindex]. Note that only these methods add styles that will export to Excel. These methods work in a similar way to [DataFrame.apply()][dfapply] and [DataFrame.map()][dfmap].\n", |
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.
If we are changing DataFrame.applymap()
to DataFrame.map()
, shouldn't we then change Styler.applymap()
to Styler.map()
to make things consistent?
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.
That's a logical next step and @attack68 has proposed in #52353 (comment) to takie on styler.applymap
after this has been merged.A good idea IMO.
Shouldn't we deprecate this before outright removing? Wouldn't this break user code for 2.1? |
That's what the current code does I think |
My mistake - totally misread that |
Yes this PR just deprecates |
dff4d63
to
415414d
Compare
9a72424
to
8bad43b
Compare
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.
Nice one! Yeah the name applymap
has confused me more than once
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.
Small comment otherwise looks good
|
Thanks,, I’ve updated. |
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.
lgtm, just a request on the whatsnew wording
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.
lgtm
Updated. |
Thanks @topper-123 |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.This PR builds on #52355, so could make sense to look at that first or ignore the first commit in this PR.