issue_comments

Thanks @mlhenderson!

Longer-term, embedding html/css may not be the way to go, but I think that falls outside the scope of this PR. We can arrange to chat with some Jupyter devs if there is interest in a discussion about longer-term solutions for the rendering in notebooks and/or JupyterLab, just let me know if you want to do that. @benbovy

Yeah, now that (since v3) Jupyterlab extensions can simply be installed using conda or pip, we may consider eventually porting this rich-repr feature into a lightweight extension instead. That would be a more robust and extensible solution. The main problem, however, is how to support the variety of notebook front-ends. There seems to be a recent agreement on refactoring Jupyter "official" front-ends (e.g., "classic" notebook) so that they all use Jupyterlab's current machinery, which will certainly help here. Not sure for the other front-ends (e.g., VSCode, GitHub, GitLab, Sphinx themes, etc.), though. Maybe we could have three repr alternatives:

• The full-featured, interactive version: compatible with all official Jupyter front-ends, maintained in a 3rd-party Jupyter extension
• A basic, html-based version (as fallback): less interactive but should work (and be nicely integrated) with all notebook front-ends without relying on ugly hacks (no css, just basic html tags)
• The plain text version

I agree that it would be worth discussing this general issue with Jupyter devs. There's more and more projects in the Python/PyData ecosystem that provide advanced, html-based reprs.

Many thanks for your thoughtful reply @mlhenderson . It seems like the proposed code is better than the current code, so I would suggest we merge, and we can iterate further if others have thoughts.

And thanks for your first contribution !

Unfortunately, I can't predict what the Jupyter project could do in the future in terms of stripping inline CSS or the html hidden attribute. I don't get the sense that this would be happening anytime soon, but I can't promise it would never happen.

I sent in the PR because I am an author of a JupyterLab extension that we plan to use at NERSC, which includes Bootstrap, and we ran into the problem of the xarray repr() returning a hidden div (blank output) in a regular notebook (trusted). Any JupyterLab extension that includes Bootstrap or has a css rule that affects how the browser treats the html hidden attribute will have similar problems with xarray in notebooks. It seemed appropriate to flag it and send in a fix before it becomes a bigger problem.

There is an existing test in xarray (which passes) that checks for the css content. If you wanted to add another test that actually spins up a Jupyter instance to test it further, that is possible but definitely more involved and may not be a fast test to run.

I think this could be merged as-is, but feedback is always welcome. If there are any suggestions for improving this PR further to resolve the existing issue please let me know.

Longer-term, embedding html/css may not be the way to go, but I think that falls outside the scope of this PR. We can arrange to chat with some Jupyter devs if there is interest in a discussion about longer-term solutions for the rendering in notebooks and/or JupyterLab, just let me know if you want to do that. @benbovy

On Fri, Nov 12, 2021 at 2:26 PM Maximilian Roos @.***> wrote:

Hi @mlhenderson https://github.com/mlhenderson — I don't know this well — if you are confident that @benbovy https://github.com/benbovy 's concern is not pressing, then we could merge? (unless @benbovy https://github.com/benbovy you are confident that it is a concern!)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/5880#issuecomment-967701381, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVNKLNE6NK653KHEUYDX5LULWICVANCNFSM5GMKPTKA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Hi @mlhenderson — I don't know this well — if you are confident that @benbovy 's concern is not pressing, then we could merge? (unless @benbovy you are confident that it is a concern!)

Thanks @mlhenderson for submitting this PR!

It looks good to me, I prefer this inline CSS workaround than the (very hacky) hidden HTML attribute workaround. My only concern is: are we sure that inline CSS won't get stripped out too in notebook front-ends in the future? (in which case the hidden solution will still work, unless it also gets removed like for GitHub's notebook renderer).

Not sure why this fix was implemented in sphinx-book-theme and not in xarray directly...

It guess it depends on which order Xarray's vs. Bootstrap's vs. Sphinx-Book's CSS is injected. Not sure about the actual order.

Unit Test Results

6 files           6 suites   52m 13s :stopwatch: 16 230 tests 14 494 :heavy_check_mark: 1 736 :zzz: 0 :x: 90 576 runs  82 396 :heavy_check_mark: 8 180 :zzz: 0 :x:

Results for commit 192a67c7.

:recycle: This comment has been updated with latest results.

@andersy005 Thanks for picking this up, and let me know if this PR needs anything else.

I looked at the other issues, and this should resolve the sphinx-book issue in https://github.com/executablebooks/sphinx-book-theme/issues/238#issuecomment-714784321 without needing a special exception for that codebase.

For https://github.com/pydata/xarray/pull/4053: Github rendered notebooks will still not look great because it will strip the inline css of the xr-wrap div (and all css). All of the actual styling (placement, color, etc) for that html block is in the css style block that gets stripped. Fixing that is a different problem from the untrusted notebook case, which is less aggressive, although that may not be the case in the future. You would need to render the content in a different format (maybe an image) for the saved notebook to get around losing css entirely.

Thank you for working on this, @mlhenderson! Seems there have been attempts at fixing this issue before

• https://github.com/pydata/xarray/pull/4053
• https://github.com/executablebooks/sphinx-book-theme/issues/238#issuecomment-714784321 which suggests using the .xr-wrap { display: block !important }. Not sure why this fix was implemented in sphinx-book-theme and not in xarray directly...

Ccing @benbovy who wrote the HTML repr in case he has any feedback.

Tests that are currently failing are unrelated to these commits and are expected to be fixed by https://github.com/pydata/xarray/pull/5875