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

nicer default styling for tables #1182

Closed
jankatins opened this issue Mar 8, 2016 · 30 comments
Closed

nicer default styling for tables #1182

jankatins opened this issue Mar 8, 2016 · 30 comments

Comments

@jankatins
Copy link
Contributor

Compared to what knitr produces for tables, the tables in the notebook look horrible:

e.g. pander tables in an R markdown document converted to html:
2016-03-08_134713

compared to similar code (the notebook doesn't understand the pandoc markdown table syntax :-( and therefore the pandoc step to convert it to html) in the jupyter notebook:

2016-03-08_135209

I also tried to include bootstrap styling (which knitr used for html), but this does not seem to work, as it seems the default styling overwrites it(?):

library(pander)
library(dplyr)
library(broom)
library(IRdisplay)
panderOptions("table.split.table", 200)
nice_pander <- function(...){
    t = capture.output(pander(...))
    mdFile <- tempfile(fileext=".md")
    cat(paste(t, collapse="\n"), file=mdFile)
    htmlFile <- knitr::pandoc(input = mdFile, format = "html")
    t = readr::read_file(htmlFile)
    #print(t)
    display_html(paste(t, collapse="\n"))
   display_html("<link rel='stylesheet' href='http://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css'>
<script src='http://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js'></script>
<script>$('tr.header').parent('thead').parent('table').addClass('table table-condensed');</script>
")
}
df <- data.frame(a=0:20, 
                 b=factor((0:20) %% 2, levels=c(0,1), labels=c("a", "b")), 
                 c=factor((0:20) %% 3, levels=c(0,1,2), labels=c("x", "y", "z")))
nice_pander(df %>% group_by(c) %>% do(tidy(t.test(a~b, data = .))))

-> It would be nice if the notebook could change the default styling for tables to something nice :-)

@flying-sheep
Copy link
Contributor

notebooks already contain bootstrap classes.

so we should probably add :not(.table) to the table styles in renderedhtml.less

@flying-sheep
Copy link
Contributor

i came up with this, but it’s only a workaround. the fix is in above comment.

library(pander)
library(dplyr)
library(broom)
library(IRdisplay)
panderOptions('table.split.table', 200)

display_html('<style>
table.table,
table.table tr,
table.table td,
table.table th { border: 0 }
</style>')

pander <- function(...) {
    t <- capture.output(pander::pander(...))
    mdFile <- tempfile(fileext = '.md')
    cat(paste(t, collapse = '\n'), file = mdFile)
    htmlFile <- suppressMessages(knitr::pandoc(input = mdFile, format = 'html'))
    t <- readr::read_file(htmlFile)
    t <- gsub('<table>', '<table class="table table-condensed">', t)
    display_markdown(paste(t, collapse = '\n'))
}

df <- data.frame(a = 0:20, 
                 b = factor((0:20) %% 2, levels = c(0,1),   labels = c("a", "b")), 
                 c = factor((0:20) %% 3, levels = c(0,1,2), labels = c("x", "y", "z")))
pander(df %>% group_by(c) %>% do(tidy(t.test(a~b, data = .))))

@jankatins
Copy link
Contributor Author

@flying-sheep Nice, thanks!

I now modified that to

bootstrapify_tables <- function(){
    display_html('<style>table.table, table.table tr, table.table td, table.table th {border: 0}
table.table-nonfluid {width: auto !important;}</style>')
    display_html('<script>
bootstrapify = function(){
   // a table which was handled by pandoc
   $("tr.header").parent("thead").parent("table").addClass("table table-condensed table-nonfluid");
   // a markdown table as it appears in a markdown cell
   $("div.text_cell div.inner_cell div.rendered_html table").addClass("table table-condensed table-nonfluid");
};
$([IPython.events]).on("notebook_loaded.Notebook", bootstrapify);
$([IPython.events]).on("kernel_ready.Kernel", bootstrapify);
$([IPython.events]).on("output_appended.OutputArea", bootstrapify);
$([IPython.events]).on("rendered.MarkdownCell", bootstrapify);
</script>')
}
bootstrapify_tables()

# ---

library(pander)
library(dplyr)
library(broom)
library(IRdisplay)
nice_pander <- function(...){
    panderOptions("table.split.table", 200)
    t = capture.output(pander(...))
    mdFile <- tempfile(fileext=".md")
    cat(paste(t, collapse="\n"), file=mdFile)
    htmlFile <- suppressMessages(knitr::pandoc(input = mdFile, format = "html"))
    t = readr::read_file(htmlFile)
    t <- gsub('<table>', '<table class="table table-condensed">', t)
    display_html(paste(t, collapse="\n"))

}

# ---

df <- data.frame(a=0:20, 
                 b=factor((0:20) %% 2, levels=c(0,1), labels=c("a", "b")), 
                 c=factor((0:20) %% 3, levels=c(0,1,2), labels=c("x", "y", "z")))
nice_pander(df %>% group_by(c) %>% do(tidy(t.test(a~b, data = .))))

Now both markdown tables and pander ones are nice...

If would be nice if the notebook contained something similar to the bootstrapify_tables event calls by default.

@minrk
Copy link
Member

minrk commented Mar 8, 2016

A PR for better default table styling would be great. I don't see anything wrong with that. I think the general scope that applies to both markdown cells and html output is .rendered_html.

@jankatins
Copy link
Contributor Author

@minrk Is there any way to only target the current inserted output? The above is run on every insert, so if there are a few tables, it will take a bit...

@minrk
Copy link
Member

minrk commented Mar 8, 2016

Not with strict CSS, I don't think, but with some Javascript you could either write style attributes or add a custom CSS class to the output div.

If you have control over the HTML output on the kernel-side, the easiest solution is probably to add a class there.

@jankatins
Copy link
Contributor Author

@minrk The current solution is based on the bootstrap table styles, which only kicks in when a class="table" is assigned to a <table>. So the current solution either needs some javascript to add that class or a change in the bootstrap css to add it unconditionally.

Is there actually a way to have a handle to the output in one of these event:

$([IPython.events]).on("output_appended.OutputArea", bootstrapify);
$([IPython.events]).on("rendered.MarkdownCell", bootstrapify);

?

@jankatins
Copy link
Contributor Author

Another strange thing is that it seems that at the time the $([IPython.events]).on("output_appended.OutputArea", bootstrapify); event is run, the function can't get a handle to the currently added html code:

funny <- function(){
    js <- '<script>
$([IPython.events]).on("output_appended.OutputArea", function(){alert($(".FUNNY_Whatever").length)});
</script>'
    IRdisplay::display_html(js)
}
funny()

and then three alerts which show 0,0,1 instead of 0,1,1

IRdisplay::display_html("<span>Not Funny</span>")
IRdisplay::display_html("<span class='FUNNY_Whatever'>Not Funny</span>")
IRdisplay::display_html("<span>Not Funny</span>")

[Chrome 49.0.2623.75 m, win7]

@jcb91
Copy link
Contributor

jcb91 commented Mar 8, 2016

@JanSchulz this (the timing issue) is down to the fact that the event is fired when markdown rendering is called, but the DOM elements are then created asynchronously, potentially after any event callbacks...
So, in short, I don't think you can't (reliably) get access to the output at the time of the callback

@jankatins
Copy link
Contributor Author

Somehow this still smells like a bug: if the event signals that something is appended, and then the should-be-appended thing isn't there, it's kind of useless :-(

@flying-sheep
Copy link
Contributor

@JanSchulz it’s hard to read your code. please add highlighting: ````r` it’s just a single letter for R!

@jcb91
Copy link
Contributor

jcb91 commented Mar 9, 2016

Well, the appended event does include some additional data (see outputarea.js#L577). The markdown rendered event is somewhat different, though, it seems. I can't say I have a complete grasp of how this works!

@jankatins
Copy link
Contributor Author

@jcb91 I looked at the arguments by which bootstrapify was called in $([IPython.events]).on("output_appended.OutputArea", bootstrapify); and couldn't makse sense of it (but my js debugging foo is beginner at best :-( -> alert(jsonify(arguments))). Is there any way to add breakpoints to inserted HTML which contain JS in chrome?

@jcb91
Copy link
Contributor

jcb91 commented Mar 9, 2016

Hmm, I'm not sure of an obvious way to do it, as it's dynamically-created javascript. I'd probably go about it by creating a small extension with a dummy function registered in the window namespace, e.g.

window.my_debugging_capturer = function () {
    console.log('my_debugging_capturer');
}

then, set a breakpoint on that function in the sources tab of chrome's devtools.

Then, the inserted HTML can call the window function, as

window.my_debugging_capturer()

to hit the breakpoint, and you can step up the stack frames in the devtools to find the callback frame... does that make sense?

@jankatins
Copy link
Contributor Author

Your idea about inserting code to go to a breakpoint brought me to the idea to google if there is a way to include a breakpoint in code (similar to the python debugger can be triggered). And yes, there is: https://stackoverflow.com/questions/10050465/set-a-javascript-breakpoint-in-code-in-chrome

Wow, thanks!

@jcb91
Copy link
Contributor

jcb91 commented Mar 9, 2016

Aha, good spot! 👍

@jankatins
Copy link
Contributor Author

And now it works:

  • Adds the stuff during the loading of the notebook -> NB which have this included have bootstrap tables after load
  • For each md render and for each output, also checks the output for tables and adds the required classes -> New content automatically gets the bootstrap styles...
  • The md tables are no left aligned (before centered) like the normal tables.
bootstrapify_tables <- function(){
    html <- '<style>table.table, table.table tr, table.table td, table.table th {border: 0;}
table.table-nonfluid {width: auto !important;}
table.table { margin-left: 0; margin-right: 0;}</style>'
    js <- '<script>
bootstrapify = function(){
   debugger;
   args = arguments;
   $(".rendered_html table").addClass("table table-condensed table-nonfluid");
   n_pandoc = $("tr.header").parent("thead").parent("table").length;
   n_md = $("div.text_cell div.inner_cell div.rendered_html table").length
   n_rendered = $(".rendered_html table").length
   console.log ("beautified "+ n_pandoc + " + " + n_md + " + " + n_rendered + " tables (pandoc + md + rendered) ...");
};
bootstrapify_output = function(){
   debugger;
   arguments[4].find( "table" ).addClass("table table-condensed table-nonfluid");
   n = arguments[4].find( "table" ).length;
   console.log ("beautified "+n+" tables in output...");
};
bootstrapify_mdcell = function(){
   debugger;
   arg2 = arguments[1];
   c = arg2.cell;
   ele = c.element;
   tbls = ele.find("table");
   tbls.addClass("table table-condensed table-nonfluid");
   n = tbls.length;
   console.log ("beautified "+n+" tables in md...");
};

$([IPython.events]).on("notebook_loaded.Notebook", bootstrapify);
$([IPython.events]).on("kernel_ready.Kernel", bootstrapify);
$([IPython.events]).on("output_appended.OutputArea", bootstrapify_output);
$([IPython.events]).on("rendered.MarkdownCell", bootstrapify_mdcell);
</script>'
    IRdisplay::display_html(html)
    IRdisplay::display_html(js)
    cat("Added bootstrap code to beautify tables...\nRemember to 'File > Trust' the notebook to make this available on the next load.")
}
bootstrapify_tables()

@jankatins
Copy link
Contributor Author

And a lesson learned: having errors in one of the js code during nb loading prevents the rest of the cells from showing up and then saving the nb destroys the notebook content for all cells after... Would probably only fixable by removing the content from the ipnb file directly...

@minrk
Copy link
Member

minrk commented Mar 10, 2016

Yikes, that really shouldn't happen. I keep fixing bugs like that, but they keep turning up.

@jankatins
Copy link
Contributor Author

@minrk The above was simple a "var is not initialized", as far as I remember like this:

bootstrapify_mdcell = function(){
   debugger;
   arg2 = arguments[1];
   c = arg2.cell;
   ele = element; // missing the `c.` in front of element
   tbls = ele.find("table");
   tbls.addClass("table table-condensed table-nonfluid");
   n = tbls.length;
   console.log ("beautified "+n+" tables in md...");
};
$([IPython.events]).on("rendered.MarkdownCell", bootstrapify_mdcell);

-> display as js, add a md table in the second cell in the nb, add more cells below, save, reload notebook, see it break :-/

@jankatins
Copy link
Contributor Author

So, I will add that js somewhere that it is run in every notebook of mine and then look what it does with pandas tables and how using styled tables affects my working ... if I get this d*** extension working ipython-contrib/jupyter_contrib_nbextensions#524 (and it works now)

@jankatins
Copy link
Contributor Author

There is now a notebook extension, which does this: ipython-contrib/jupyter_contrib_nbextensions#524

@gnestor gnestor closed this as completed Sep 14, 2016
@minrk
Copy link
Member

minrk commented Sep 14, 2016

@gnestor while there is an extension, I think there's still plenty of room for improving the default table style. @JanSchulz do you want to make a PR based on your experience with the extension so far?

@minrk minrk reopened this Sep 14, 2016
@minrk minrk modified the milestones: backlog, no action Sep 14, 2016
@minrk
Copy link
Member

minrk commented Sep 14, 2016

Weird, this Issue should have the wishlist milestone, but it appears to be gone. Marking as backlog for now, though I don't know what that means.

@gnestor
Copy link
Contributor

gnestor commented Sep 14, 2016

@minrk Ok, sorry for assuming it had been resolved.

@ellisonbg renamed the wishlist milestone to backlog (as in agile development backlog) for this repo and ipywidgets.

@minrk
Copy link
Member

minrk commented Sep 14, 2016

OK. I guess we should add the wishlist milestone back, then? To me, backlog (not dealt with yet, needs triage) doesn't mean what we meant by wishlist (no immediate plans to deal with, volunteer contribution welcome). In fact, it seems a bit opposite in terms of what it means for where I should be focusing my attention.

@jankatins
Copy link
Contributor Author

@minrk Sorry, I won't have time in the near future to work on this :-/

@TomAugspurger
Copy link

I don't have much time at the moment either, but for some guidance on what would be nice from pandas' prospective. Ideally with this small patch to pandas

diff --git a/pandas/formats/format.py b/pandas/formats/format.py
index e508998..fa866b9 100644
--- a/pandas/formats/format.py
+++ b/pandas/formats/format.py
@@ -992,7 +992,7 @@ class HTMLFormatter(TableFormatter):
         indent = 0
         frame = self.frame

-        _classes = ['dataframe']  # Default class.
+        _classes = ['dataframe', 'table', 'table-condensed']  # Default class.
         if self.classes is not None:
             if isinstance(self.classes, str):
                 self.classes = self.classes.split()

we'd get the nicely formatted output. With that patch, I see

screen shot 2016-09-14 at 4 46 55 pm

From poking around most of the styling comes from .rendered_html tr, .rendered_html th, .rendered_html td, .rendered_html table, tr. Hope that helps.

@gnestor
Copy link
Contributor

gnestor commented Sep 20, 2016

I'm working on PR to update table styles to match JupyterLab: #1776

@takluyver
Copy link
Member

Closing as #1776 improved the default table style.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants