-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
ENH: Adds ability to store tooltips as title attribute through pandas styler #56981
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: Adds ability to store tooltips as title attribute through pandas styler #56981
Conversation
When I run ruff locally, on the files I changed, there are no changes made. edit: Too fast to comment. |
Getting back to this tonight, will address all comments. |
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, much happier after the removal of regex and sticking with existing pattern.
couple of quick suggestions, and it looks like a merge gone wrong somewhere, there are a few dangling unrelated file changes. Probably just needs final merge to master, then I will look to approve,.
Co-authored-by: JHM Darbyshire <24256554+attack68@users.noreply.github.com>
Do I have to worry about that Numpy failure? |
No I dont think so it looks unrelated. And if on the off chance it is not it is probably worth fixing in a follow up PR. |
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, you still have some floating black
issues that add commas in random files. Perhaps a re-merge necessary as one final thing.
@rhshadrach @mroeschke you OK to merge this is?
Co-authored-by: Matthew Roeschke <10647082+mroeschke@users.noreply.github.com>
If I merge main one more time, am I good? |
Tests are passing and dangling 'comma' changes have gone. I think this is ready now. |
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 merge when ready @rhshadrach
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 the changes, lgtm!
Thanks @Delengowski! |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.