ENH: Style blocks by TomAugspurger · Pull Request #15954 · pandas-dev/pandas (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

ENH: Add blocks to Styler template

This will make subclassing the Styler and extending
the templates easier.

REF: Move template to its own file

Use Environment and PackageLoader to load it

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all for the reorg, do we need this in the top-level namespace?

maybe make a sub-dir for the style things at some point (e.g. pandas/formats/style (but another PR if you think its worth it).

- ``DataFrame.to_excel()`` has a new ``freeze_panes`` parameter to turn on Freeze Panes when exporting to Excel (:issue:`15160`)
- ``pd.read_html()`` will parse multiple header rows, creating a multiindex header. (:issue:`13434`).
- HTML table output skips ``colspan`` or ``rowspan`` attribute if equal to 1. (:issue:`15403`)
- ``pd.Styler`` template now has blocks for easier extension (:issue:`15649`)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the right one? (maybe use a :class:`....` ?

from pandas.util.print_versions import show_versions
from pandas.io.api import *
from pandas.util._tester import test
from pandas.formats.style import Styler

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you really want to add this to the top-level? (does this import stuff)?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think it needs to be importable somewhere 'public', not sure if top level is the right spot, could argue either way. Do we need a pandas.api.io?

@@ -0,0 +1,61 @@
{%- block before_style -%}{%- endblock before_style -%}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure this is in the MANIFEST.IN

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, only one significant comment on structure of template

{%- endblock thead %}
{%- block tbody %}
<tbody>
{%- for r in body %}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a rows block here? (or alternatively empty before_rows and after_rows). What I was trying to do is add additional rows at the end of the table which I don't think is possible with these extension points.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a before_rows, after_rows and tr blocks. Here's the new structure

from pandas.util.print_versions import show_versions
from pandas.io.api import *
from pandas.util._tester import test
from pandas.formats.style import Styler

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think it needs to be importable somewhere 'public', not sure if top level is the right spot, could argue either way. Do we need a pandas.api.io?

def render(self, **kwargs):
"""
Render the built up styles to HTML

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add kwargs behavior to docstring

From the notebook

For convenience, we provide the styler_factory function that does the same as the custom subclass.

styler_factory -> from_custom_template

Thanks. About the API: I don't really want it at the top level but it has to be public if people are subclassing. I just wasn't sure how we resolved the public API and submodules issue :)

…

On Apr 9, 2017, at 8:03 AM, chris-b1 ***@***.***> wrote: From the notebook For convenience, we provide the styler_factory function that does the same as the custom subclass. styler_factory -> from_custom_template — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

I do think it needs to be importable somewhere 'public', not sure if top level is the right spot, could argue either way. Do we need a pandas.api.io?

yes I think this is the way to go.

Do we need a pandas.api.io?

We already have a publicly accessible pandas.io, so if we want it in io, I would put it there.

We already have a publicly accessible pandas.io, so if we want it in io, I would put it there.

Yeah, having both would be confusing. Are people ok with pandas.io.Styler? (or would it be pandas.io.api.Styler?)

jinja2 being optional makes putting this in the namespace a bit strange. Happy to hear suggestions on ways to make that / the API tests better :/

PackageLoader, Environment, ChoiceLoader, FileSystemLoader
)
except ImportError:
msg = "pandas.Styler requires jinja2. "\

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try:
     from jinja2 .....
except ImportError:
     class Styler(ImportError):
            pass
     return

will give you a Styler but if you try to use it and jinja2 is not installed will raise I think you can give it a nice message a well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jreback didn't quite work since you can't early-return from a module.

However, this does work:

try: from pandas.formats.style import Styler except: from pandas.compat import add_metaclass

# We want to *not* raise an ImportError upon init
# We *do* want to raise an ImportError with a custom message
# when the class is instantiated or subclassed.

_msg = ("pandas.io.api.Styler requires jinja2. "
        "Please install with `conda install Jinja2` "
        "or `pip install Jinaj2`")

class _Check(type):
    """Raises import error upon subclassing."""
    def __init__(cls, name, bases, clsdict):
        if len(cls.mro()) > 2:
            raise ImportError(_msg)
        super(_Check, cls).__init__(name, bases, clsdict)

@add_metaclass(_Check)
class Styler(object):
    def __init__(self, *args, **kargs):
        raise ImportError(_msg)

Are we ok with a metaclass, just for a nice error message? :D

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh if that works great.

@@ -0,0 +1,2 @@
""" Public IO API """
from pandas.formats.style import Styler # noqa

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with having a pandas.api.io, but isn't somewhat the opposite of the recommendation in #13634? Which is the top level modules would be 'public' and internals moved into core - so this should be exposed as pd.io.Styler? (I could be misunderstanding that issue)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pandas.io is going to stay, so importing from pandas.io is fine.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, thought I removed that. Also might have just messed up my rebase. This isn't supposed to be there. It's supposed to be pandas.io.api

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be just be pandas.io? The second level api are import * into the top level.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pandas.io.api imports * to pandas namespace; so you could make Styler just show up there (or you can leave the from pandas.io.api import * and just del Styler (prob shorter)

Use Environment and PackageLoader to load it

TODO: packaging, init.py?

ENH: Add blocks to Styler template

This will make subclassing the Styler and extending the templates easier.

Jinja2 is optional, so Styler is maybe not there

Nicer import failure

Fixed rebase

As I said above, we already have public things in pandas.io, so I don't really see the point of creating a new pandas.api.io ?

Currently, our API documentation lists it as pandas.formats.style.Styler (so when moving, we should deprecate that). So one possibility would be to move the full formats module into pandas.io ? Or move to core and only expose the public ones in pandas.io (or in pandas.io.formats / pandas.io.style).

If we decide to move the formats module (which would certainly be for another PR), we could also just leave Styler where it is for now in this PR, and do the move / change of locations / deprecation in the other PR.

OK, should be good now. Sorry about that.

I am shortly going to move a bunch of things around. But pandas.io is going to stay (pandas.formats will move).

you could move .style under .io if you want.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

api.rst will need an update as well, depending on what we decide

-------
rendered: str
the rendered HTML
**kwargs:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to make the docstring a raw string (or escape the *'s), otherwise sphinx will complain :-)

- ``DataFrame.to_excel()`` has a new ``freeze_panes`` parameter to turn on Freeze Panes when exporting to Excel (:issue:`15160`)
- ``pd.read_html()`` will parse multiple header rows, creating a multiindex header. (:issue:`13434`).
- HTML table output skips ``colspan`` or ``rowspan`` attribute if equal to 1. (:issue:`15403`)
- ``pd.io.api.Styler`` template now has blocks for easier extension (:issue:`15649`)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have an example in the notebook I suppose. Is it possible to put a link here to such an example? Not sure how it works with nbsphinx included files to put links to specific sections

You have an example in the notebook I suppose. Is it possible to put a link here to such an example? Not sure how it works with nbsphinx included files to put links to specific sections

@jorisvandenbossche yeah that worked! It's just

:ref`text <notebook.ipynb#section-title>`

# We do want to raise an ImportError with a custom message
# when the class is instantiated or subclassed.
@_add_metaclass(_UnSubclassable)
class Styler(object):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this jinja2 ?

'SparseSeries', 'TimeGrouper', 'Timedelta',
'TimedeltaIndex', 'Timestamp', 'Interval', 'IntervalIndex']
'TimedeltaIndex', 'Timestamp', 'Interval', 'IntervalIndex',
'Styler']

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can actually del Styler from pandas namespace

ipywidgets I think is needed in doc-build.

Whoops, looked right past that. Yeah I guess they've split that out now.

I can do now, or in a separate PR since all the checks already passed here.

either way
or can add and push then merge

from pandas.io.gbq import read_gbq
try:
from pandas.formats.style import Styler
except:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ImportError

Opened a follow-up issue for the remaining issues that were being discussed: #16009

This was referenced

Apr 15, 2017

ENH: Style blocks by TomAugspurger · Pull Request #15954 · pandas-dev/pandas (original) (raw)

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels