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

register custom DisplayFormatter for table schema #16198

Merged
merged 3 commits into from
May 2, 2017

Conversation

minrk
Copy link
Contributor

@minrk minrk commented May 2, 2017

instead of using _ipython_display_ for custom mime-types

This should solve the issues in #16171, without needing to register/unregister _ipython_display_.

In the long run, ipython/ipython#10496 should remove the need for any custom registration, in favor of a simple _repr_mimebundle_ method.

I think the only thing missing is the UnserializableWarning because IPython does its own catching and displaying of tracebacks when formatters fail. I can copy and modify a little more code out of the base Formatter to preserve that warning, though.

instead of using `_ipython_display_` for custom mime-types
from IPython.core.formatters import BaseFormatter

class TableSchemaFormatter(BaseFormatter):
print_method = '_repr_table_schema_'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm beginning to wonder if we should be calling this repr_data_resource, based on the mimetype we ended up at (which includes the schema inside)

Copy link
Contributor

Choose a reason for hiding this comment

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

The idea being "table schema" refers to just the schema? Makes sense to me.

@TomAugspurger
Copy link
Contributor

About the UnserializeableWarning, what's the behavior from the PR when pandas _repr_table_schema_ fails?

For background, some DataFrames just can't be serialized according to the table schema spec. In these cases, we'd ideally fall back to _repr_html_.

@TomAugspurger
Copy link
Contributor

For background, some DataFrames just can't be serialized according to the table schema spec. In these cases, we'd ideally fall back to repr_html.

Oh neat, it looks like it falls back to the next formatter. That's perfect.

So we can just get rid of the UnserializableWarning altogether then (I added it just for this). I can take care of that if you want.

@minrk
Copy link
Contributor Author

minrk commented May 2, 2017

About the UnserializeableWarning, what's the behavior from the PR when pandas repr_table_schema fails?

You'll get a traceback (not user-mute-able like a warning) with the failure and other reprs will still display. If that's okay, I'll just remove the warning.


expected = {'text/plain', 'text/html'}
assert set(arg.keys()) == expected
assert set(formatted[0].keys()) == expected
assert "orient='table' is not supported for MultiIndex" in (
Copy link
Contributor

Choose a reason for hiding this comment

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

Will have to remove this line.

with pd.option_context('display.html.table_schema', True):
assert 'application/vnd.dataresource+json' in formatters
assert formatters[mimetype].enabled

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 our linter complained about whitespace here.

reprs = {"text/plain": text, "text/html": html, "text/latex": latex,
"application/vnd.dataresource+json": table_schema}
reprs = {k: v for k, v in reprs.items() if v}
display(reprs, raw=True)

Copy link
Contributor

Choose a reason for hiding this comment

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

@TomAugspurger
Copy link
Contributor

@minrk I did those minor fixes. Thanks for putting this together. Feels much better than monkey patching :)

@codecov
Copy link

codecov bot commented May 2, 2017

Codecov Report

Merging #16198 into master will decrease coverage by <.01%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16198      +/-   ##
==========================================
- Coverage   90.86%   90.86%   -0.01%     
==========================================
  Files         162      162              
  Lines       50871    50867       -4     
==========================================
- Hits        46225    46221       -4     
  Misses       4646     4646
Flag Coverage Δ
#multiple 88.65% <94.44%> (-0.01%) ⬇️
#single 40.31% <5.55%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/generic.py 91.67% <ø> (+0.03%) ⬆️
pandas/errors/__init__.py 100% <ø> (ø) ⬆️
pandas/core/config_init.py 94.37% <94.44%> (-0.15%) ⬇️
pandas/core/common.py 90.68% <0%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20fda22...760892a. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented May 2, 2017

Codecov Report

Merging #16198 into master will decrease coverage by <.01%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16198      +/-   ##
==========================================
- Coverage   90.86%   90.86%   -0.01%     
==========================================
  Files         162      162              
  Lines       50871    50867       -4     
==========================================
- Hits        46225    46221       -4     
  Misses       4646     4646
Flag Coverage Δ
#multiple 88.65% <94.44%> (-0.01%) ⬇️
#single 40.31% <5.55%> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/generic.py 91.67% <ø> (+0.03%) ⬆️
pandas/errors/__init__.py 100% <ø> (ø) ⬆️
pandas/core/config_init.py 94.37% <94.44%> (-0.15%) ⬇️
pandas/core/common.py 90.68% <0%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20fda22...760892a. Read the comment docs.

@TomAugspurger TomAugspurger merged commit 39cc1d0 into pandas-dev:master May 2, 2017
@TomAugspurger
Copy link
Contributor

Thanks Min!

@minrk
Copy link
Contributor Author

minrk commented May 3, 2017

Thanks, @TomAugspurger!

@minrk minrk deleted the table-schema-formatter branch May 3, 2017 07:49
pcluo pushed a commit to pcluo/pandas that referenced this pull request May 22, 2017
)

* register custom DisplayFormatter for table schema

instead of using `_ipython_display_` for custom mime-types

* remove unused UnserializableWarning

* PEP8 fixes
@TomAugspurger TomAugspurger added this to the No action milestone May 27, 2017
yarikoptic added a commit to neurodebian/pandas that referenced this pull request Jul 12, 2017
Version 0.20.0

* tag 'v0.20.0': (742 commits)
  RLS: v0.20.0
  DOC: Whatsnew cleanup (pandas-dev#16245)
  TST: Test CategoricalIndex in test_is_categorical (pandas-dev#16243)
  TST: xfail some bottleneck on windows (pandas-dev#16240)
  DOC, TST: Document and Test Functions in dtypes/common.py (pandas-dev#16237)
  TST: Remove __init__ statements in testing (pandas-dev#16238)
  DOC: don't include all methods/attributes of IntervalIndex (pandas-dev#16221)
  PKG: Fix ModuleNotFoundError: No module named 'pandas.formats' (pandas-dev#16239)
  RLS: v0.20.0rc2
  CLN: make submodules of pandas.util private (pandas-dev#16223)
  MAINT: Remove tm.TestCase from testing (pandas-dev#16225)
  MAINT: Complete Conversion to Pytest Idiom (pandas-dev#16201)
  DOC: add whatsnew for 0.21.0
  DEPR: correct deprecation message for datetools (pandas-dev#16202)
  API Change repr name for table schema (pandas-dev#16204)
  DOC: Remove various warnings from doc build (pandas-dev#16206)
  DOC: add whatsnew for v0.20.1
  BUG: Fixed renaming of falsey names in build_table_schema (pandas-dev#16205)
  COMPAT: ensure proper extension dtype's don't pickle the cache (pandas-dev#16207)
  REF: register custom DisplayFormatter for table schema (pandas-dev#16198)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants