Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ENH: Add custom descriptors (such as dtype, nunique, etc.) to Styler output #43894
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
ENH: Add custom descriptors (such as dtype, nunique, etc.) to Styler output #43894
Changes from 89 commits
9743099
8a0253e
74c418e
70535c5
2fbe569
7903723
6a2793c
d227914
7ca5002
f27f7ed
a1000a7
c44dcda
c4c9aaa
c22cf0d
0e0b46e
1f3bbec
021bc26
4ba3dff
7fee05d
baa3233
f4ad390
22b03e3
db214d8
771d056
24952ae
1d47d0f
566738d
230138a
b9ba9ea
ea2bba1
75de033
4c34bc2
68ee832
ca70491
7a1994e
91320f8
92c1941
aad0e16
61b24ed
d4c5715
aa0172f
84a814f
f06b727
131070f
a048122
4931f32
e9716f2
d983464
c08ef82
0f11a62
5c52957
17d181c
e5b4d3e
f98ba9f
0cc5502
c5b75cc
ac1384b
c948f11
afa8f10
60efbbb
9ef5b44
a02b4ac
6b51d88
814bace
33c35a4
91d8680
260d356
46856fb
a4dce4d
fefb420
f2c8f0e
a945da5
b3c0f96
f36d4fd
e78907f
cd085df
45f5879
6f64e80
1c5cf53
58aa894
bd2fe7d
02aeae5
5541396
beb2bc1
7d62bb3
6bfb5a9
9554f89
d8e11c8
4f935c4
7031897
0c034a6
6128ccb
bb8201e
44204e0
ebec1d6
c205c38
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Could you some additional details about the callable. Specifically it must (or should) return a scalar quantity. Is the callable allowed to throw and exception? If so, what happens? I would be good to clarify this.
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.
The screen show shows no lable on this line? Was it not possible to have
.__name__
as a default test?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.
Is this a common API? I think most places where alternative input are allowed either use a Sequence of some sort or
dict[str, str | Callable]
. Would be be cleaner if the allowed inputs wereSequence[str | Callable] | dict[str | Callable]
.Also, can't the
Callable be better described? Isn't is
Callable[[Series], float]? It seems that the
Callablemust take a
Seriesand return some sort of scalar value. Thie question is whether the scalar value can be non-numeric. If it could be any numeric value, than you could you use something like
Callable[[Series], int | float]. So more generally, what are the requirements for the
Callable`?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.
The Callable could techincally return anything so long as the returned object has a
__str__
representation to populate in the HTML element. Common values might be int, float, string, but yes it does accept only series.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 think the sig should then be
Callable[[Series],Any]
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.
List[Tuple[str, str]]
is present for some of theStyler
functionsThere 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 you do stick with the tuple, it should be
tuple(str, str | Callable)
which would allow someone to pass a custom name for the output along with a common function, e.g.,("average", "mean"). If moving to a dict, then both
{"average":"mean"}and
{"average":Series.mean}` should be allowed.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.
yes, agreed, having a think if this can be improved.
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.
What kind of tuples are allowed? Would be better to be as specific as possible to reduce any future refactor risks.
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.
When the descriptor is a
Callable
I think it would be best to trygetattr(descriptor, "name", None)
for thename
rather than committing to a blank name. For examplepd.Series.mean.__name__
ismean
.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.
"<lambda>"
i thought was worth blankingThere 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.
Why not be strict here and raise?
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 the interest of a simpler initial PR i did not include a subset argument so the function needs to work on all colums. some dtypes might bot be suitable for the function so best approach is silent raise i think. descriptors aim to be more helpful than precise, so i think raising when failing would be annoying
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 see. What you think about raising a
UserWarning
that applyingfunc
failed for a column? Would that be too noisy?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 still think so, but even if it wasn't, the way to turn it off is difficult:
show_warnings
kwarg would have to be saved inStyler
namespace and evaluated upon render: still a bit messy.However, with you prompting me to think about it, probably means better documentation and better examples could highlight this and make the behaviour (and workarounds) clear.