-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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
WIP: REF: DataFrame.to_html
directs to Styler.to_html
#43161
Conversation
Seems like some great work is being done here. I'm likely missing something but has anyone else in @pandas-dev/pandas-core approved/+1 this? As I suspect this might receive some complaints from users for 1.4 since this is such a commonly used method. |
What goal does it serve to have users reach deeper to access a method that has been around for ages? |
we could leave this method snd just return the styler inpl of it? (and if there are specific deprecations then do those for unsupported features?) |
@attack68 I seem to remember some previous discussion about this, but it's not in the issue you link (#41693). Do you know of older issues that had some of this? (#41693 does summarize some concerns that I suppose came from earlier discussions) (eg issues like having jinja2 as a requirement for html repr in the notebook, which basically makes jinja2 a semi-required dependency) EDIT: one such issue might be #11700 |
Yes @jorisvandenbossche, in that issue you mentioned two barriers, which have been addressed. You identified same advantages as me, and mentioned disadvantages; the performance is resolved, you can test but I think
@jreback yes for sure, but as mentioned would need a refactoring of the input arguments, I think. |
so i think would be ok to deprecate these scenarios
then we can remove the existing implementation in the future (though keep .to_html is prob ok and just have it call the styler inpl) |
DataFrame.to_html
in favour of Styler.to_html
DataFrame.to_html
directs to Styler.to_html
I wan't suggesting keeping 2 implementation. I was just quibbling moving |
will work towards that, seems sensible and has good support. |
However, I think we need to discuss the jinja2 dependency a bit more, first. What does that mean? Do we keep that as an optional dependency as it is right now? |
Personally, I expected that jinja2 would be added as a hard requirement. Having said that using pandas in a Notebook is commonly used with charting, where matplotlib is an optional dependency, so forcing another optional dependency might not be too restrictive, especially if well documented. A fallback for a non-jinja2 notebook could be to use the string representation of a DataFrame with/without a warning (although currently it doesnt seem to trim for large dfs)? |
so adding jinja2 as a hard requirement for styler is fine i think |
ahh just read @jorisvandenbossche comment again i think we should just add it as a hard requirement |
Quick question: Would this be targeted for 1.4 or 2.0? I feel like we would need a deprecation cycle before switching the implementations around. |
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
closing this temporarily in favour of #44451 |
This is being worked towards in #41693. Please look at that issue and comment if there are specific features we need to keep/discuss. This is a medium term project I've been working on but would be great if someone else in team can be sounding board.