Skip to content
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

Closed

Conversation

TomAugspurger
Copy link
Contributor

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)

@TomAugspurger
Copy link
Contributor Author

Still a WIP for now, need to add handling for subset for some cases. Wanted to get this up since it's come up a couple times.

@TomAugspurger TomAugspurger changed the title Style display format WIP: Style display format Jan 28, 2016
@jreback jreback added Output-Formatting __repr__ of pandas objects, to_string IO HTML read_html, to_html, Styler.apply, Styler.applymap labels Jan 28, 2016
@jreback jreback added this to the 0.18.0 milestone Jan 28, 2016
@TomAugspurger
Copy link
Contributor Author

Ok, ready for review whenever someone gets a chance. These changes should be entirely backwards compatible.


Returns
-------
self: Styler
Copy link
Contributor

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

@TomAugspurger
Copy link
Contributor Author

Thanks Jeff.

I fixed everything except the select_dtypes enhancement. Do that as a followup since presumably it would be for all the style methods, not just format?

@jreback
Copy link
Contributor

jreback commented Jan 31, 2016

did u update the docs with an example for this?

@TomAugspurger
Copy link
Contributor Author

Yeah, I've uploaded a gist. I'm not sure why the diff was so badly mangled.

I looked into nbsphinx a bit and it looks promising. We'll need to tweak a few things to get it working with our site. I plan on doing that sometime.

if not callable(formatter):
return lambda x: formatter.format(x)
else:
return formatter
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@TomAugspurger TomAugspurger changed the title WIP: Style display format Style display format Feb 6, 2016
@TomAugspurger TomAugspurger force-pushed the style-display-format branch 2 times, most recently from 192080b to aee0d90 Compare February 7, 2016 15:58
@TomAugspurger
Copy link
Contributor Author

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)
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor Author

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.

@TomAugspurger
Copy link
Contributor Author

I need to figure out why the doc build failed here. Haven't been able to reproduce it locally yet.

@jreback
Copy link
Contributor

jreback commented Feb 10, 2016

lgtm. merge when ready.

@TomAugspurger
Copy link
Contributor Author

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.

@jreback
Copy link
Contributor

jreback commented Feb 12, 2016

still bombing sphinx?

@TomAugspurger
Copy link
Contributor Author

Surprisingly, yes. Even though I removed my changes to the docs. I've got one more thing to try out this morning.

On Feb 11, 2016, at 21:14, Jeff Reback notifications@github.com wrote:

still bombing sphinx?


Reply to this email directly or view it on GitHub.

@jorisvandenbossche
Copy link
Member

@TomAugspurger Looking at the doc build log, it gives an error with "ImportError: No module named abc" (for the from collections.abc import MutableMapping that you added). That can possibly explain also all the other problems with sphinx.
I get that same error message if I just try the import, but strange that the other tests do not fail. (a python 2 vs 3 issue?)

@@ -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):
'''
Copy link
Member

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)

@TomAugspurger TomAugspurger force-pushed the style-display-format branch 2 times, most recently from b2870a4 to 1c52491 Compare February 12, 2016 13:17
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
@TomAugspurger
Copy link
Contributor Author

Thanks Joris! Hopefully that was it. It is strange that the tests didn't fail on py27. We do skip on the 27_slow_nnet_LOCALE job, since we had a strange interaction between Jinja and matplotlib. Not sure why the other ones didn't fail.

@jorisvandenbossche
Copy link
Member

Yeah, very strange .. I checked and they are certainly not skipped on some of the python 2.7 builds

@TomAugspurger
Copy link
Contributor Author

@jreback passed without breaking sphinx, if you want to merge.

@jreback
Copy link
Contributor

jreback commented Feb 12, 2016

gr8!

thanks!

and of course can just join the travis queue :)

@jreback jreback closed this in cf8b7f8 Feb 12, 2016
@jreback
Copy link
Contributor

jreback commented Feb 12, 2016

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.

@TomAugspurger
Copy link
Contributor Author

Ahh yeah, sorry. I've made a todo item.

TomAugspurger added a commit that referenced this pull request Mar 3, 2016
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 <tom.w.augspurger@gmail.com>
Author: Henry Hammond <henryhhammond92@gmail.com>

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
TomAugspurger added a commit to TomAugspurger/pandas that referenced this pull request Mar 4, 2016
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.
jreback pushed a commit that referenced this pull request Mar 5, 2016
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.
@TomAugspurger TomAugspurger deleted the style-display-format branch November 3, 2016 12:38
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO HTML read_html, to_html, Styler.apply, Styler.applymap Output-Formatting __repr__ of pandas objects, to_string
Projects
None yet
3 participants