-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Style display format #12162
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
Style display format #12162
Conversation
Still a WIP for now, need to add handling for |
ea8ddc6
to
75b2570
Compare
Ok, ready for review whenever someone gets a chance. These changes should be entirely backwards compatible. |
|
||
Returns | ||
------- | ||
self: Styler |
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 maybe update the other doc-strings (.apply
, .applymap
) etc to the same Returns
(e.g. self : Styler
) is pretty clear
75b2570
to
ecf6362
Compare
Thanks Jeff. I fixed everything except the |
did u update the docs with an example for this? |
ecf6362
to
849ae28
Compare
if not callable(formatter): | ||
return lambda x: formatter.format(x) | ||
else: | ||
return formatter |
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.
doesn't this have to be a string?
e.g. df.style.format(5)
is a failure, yes? I suppose that is like .set_precision(5)
?
maybe raise here if its not a callable or string?
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.
Good call. I now raise if it's not a callable or string.
This is probably something we should do more of. Since we compute the styles lazily, it'd be best to raise early when we can.
192080b
to
aee0d90
Compare
All green here. |
def _maybe_wrap_formatter(formatter): | ||
if not (callable(formatter) or com.is_string_like(formatter)): | ||
msg = "Expected a template string or callable, got {} instead".format( | ||
formatter) |
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.
ok, though I would have simply done an if/elsif/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.
if you want to fix, then go ahead and merge (use the scripts/merge-pr.py
tool. note that I will occasionaly edit the generated commit message if it includes too much stuff (e.g. it by default includes everything at the top of the PR). you can do it before it pushes upstream.
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.
Will fix and merge tonight.
aee0d90
to
b8e9d05
Compare
I need to figure out why the doc build failed here. Haven't been able to reproduce it locally yet. |
lgtm. merge when ready. |
I'll get all my PRs finished up tonight. If I can't figure out why this is causing the sphinx build to fail I'll drop the changes to the notebook and do that in a followup PR. |
b8e9d05
to
60eb3ef
Compare
still bombing sphinx? |
Surprisingly, yes. Even though I removed my changes to the docs. I've got one more thing to try out this morning.
|
@TomAugspurger Looking at the doc build log, it gives an error with "ImportError: No module named abc" (for the |
@@ -255,6 +272,68 @@ def _translate(self): | |||
precision=precision, table_styles=table_styles, | |||
caption=caption, table_attributes=self.table_attributes) | |||
|
|||
def format(self, formatter, subset=None): | |||
''' |
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.
Can you use """
here instead of '''
? Does not really matter of course, but I just like some uniformity on this :-) (and I think we mainly use the double ones for docstrings)
b2870a4
to
1c52491
Compare
Closes pandas-dev#11692 Closes pandas-dev#12134 Closes pandas-dev#12125 This adds a `.format` method to Styler for formatting the display value (the actual text) of each scalar value. In the processes of cleaning up the template, I close pandas-dev#12134 (spurious 0) and pandas-dev#12125 (KeyError from using iloc improperly) cherry pick test from pandas-dev#12126 only allow str formatting for now fix tests for new spec formatter callable update notebook
1c52491
to
a3c38fe
Compare
Thanks Joris! Hopefully that was it. It is strange that the tests didn't fail on py27. We do skip on the |
Yeah, very strange .. I checked and they are certainly not skipped on some of the python 2.7 builds |
@jreback passed without breaking sphinx, if you want to merge. |
gr8! thanks! and of course can just join the travis queue :) |
thanks @TomAugspurger I realized after I pressed Y that we don't have all of the issues listed in the whatsnew. Can you add them when you have a chance. |
Ahh yeah, sorry. I've made a todo item. |
Addresses #12090 (comment) by making the `Styler` behaviour match regular `to_html` behaviour. This PR is based from #12162. ##### New behaviour <img width="596" alt="screen shot 2016-02-08 at 10 35 23 am" src="https://cloud.githubusercontent.com/assets/3064019/12890011 /b183e81e-ce4f-11e5-9b9f-c021fcb33c5a.png"> cc @TomAugspurger Author: Tom Augspurger <[email protected]> Author: Henry Hammond <[email protected]> Closes #12260 from HHammond/style-remove-col-index-none and squashes the following commits: 48c2f4c [Henry Hammond] BUG: Ignore style column headers when None a15248a [Tom Augspurger] ENH: display_format for style
Also redoes some of the changes that were inadvertently revereted in pandas-dev#12260. That PR had two commits, one of which was an older version of what was eventually merged in pandas-dev#12162. The stale commit incorrectly merged was a15248a. It should have been a3c38fe. For the most part the changes were just style, but there was a python2 import error, which our tests didn't fail on because I was trying to catch a jinja ImportError, and instead caught all ImportErrors.
Also redoes some of the changes that were inadvertently revereted in #12260. That PR had two commits, one of which was an older version of what was eventually merged in #12162. The stale commit incorrectly merged was a15248a. It should have been a3c38fe. For the most part the changes were just style, but there was a python2 import error, which our tests didn't fail on because I was trying to catch a jinja ImportError, and instead caught all ImportErrors.
Closes #11692
Closes #12134
Closes #12125
This adds a
.format
method to Styler for formatting the display value(the actual text) of each scalar value.
In the processes of cleaning up the template, I close #12134 (spurious 0)
and #12125 (KeyError from using
iloc
improperly)