issue_comments

Table actions

5 rows where author_association = "CONTRIBUTOR" and issue = 1031724989 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

Suggested facets: created_at (date), updated_at (date)

issue 1

  • Alternate method using inline css to hide regular html output in an untrusted notebook · 5 ✖

author_association 1

  • CONTRIBUTOR · 5 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
967729952 https://github.com/pydata/xarray/pull/5880#issuecomment-967729952 https://api.github.com/repos/pydata/xarray/issues/5880 IC_kwDOAMm_X845rmMg mlhenderson 2807085 2021-11-12T23:41:15Z 2021-11-12T23:41:15Z CONTRIBUTOR

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.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Alternate method using inline css to hide regular html output in an untrusted notebook 1031724989
947982100 https://github.com/pydata/xarray/pull/5880#issuecomment-947982100 https://api.github.com/repos/pydata/xarray/issues/5880 IC_kwDOAMm_X844gQ8U github-actions[bot] 41898282 2021-10-20T19:44:04Z 2021-10-24T08:27:56Z CONTRIBUTOR

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.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Alternate method using inline css to hide regular html output in an untrusted notebook 1031724989
948914260 https://github.com/pydata/xarray/pull/5880#issuecomment-948914260 https://api.github.com/repos/pydata/xarray/issues/5880 IC_kwDOAMm_X844j0hU mlhenderson 2807085 2021-10-21T19:00:07Z 2021-10-21T19:00:07Z CONTRIBUTOR

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

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Alternate method using inline css to hide regular html output in an untrusted notebook 1031724989
948912399 https://github.com/pydata/xarray/pull/5880#issuecomment-948912399 https://api.github.com/repos/pydata/xarray/issues/5880 IC_kwDOAMm_X844j0EP mlhenderson 2807085 2021-10-21T18:57:17Z 2021-10-21T18:57:17Z CONTRIBUTOR

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.

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Alternate method using inline css to hide regular html output in an untrusted notebook 1031724989
947985498 https://github.com/pydata/xarray/pull/5880#issuecomment-947985498 https://api.github.com/repos/pydata/xarray/issues/5880 IC_kwDOAMm_X844gRxa mlhenderson 2807085 2021-10-20T19:49:27Z 2021-10-20T19:49:27Z CONTRIBUTOR

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

 
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
   Alternate method using inline css to hide regular html output in an untrusted notebook 1031724989

Advanced export

JSON shape: default, array, newline-delimited, object

CSV options: 

CREATE TABLE [issue_comments] (
   [html_url] TEXT,
   [issue_url] TEXT,
   [id] INTEGER PRIMARY KEY,
   [node_id] TEXT,
   [user] INTEGER REFERENCES [users]([id]),
   [created_at] TEXT,
   [updated_at] TEXT,
   [author_association] TEXT,
   [body] TEXT,
   [reactions] TEXT,
   [performed_via_github_app] TEXT,
   [issue] INTEGER REFERENCES [issues]([id])
);
CREATE INDEX [idx_issue_comments_issue]
    ON [issue_comments] ([issue]);
CREATE INDEX [idx_issue_comments_user]
    ON [issue_comments] ([user]);