home / github

Menu
  • Search all tables
  • GraphQL API

issue_comments

Table actions
  • GraphQL API for issue_comments

27 rows where author_association = "MEMBER" and issue = 510294810 sorted by updated_at descending

✎ View and edit SQL

This data as json, CSV (advanced)

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

user 8

  • shoyer 10
  • benbovy 9
  • choldgraf 2
  • dcherian 2
  • mrocklin 1
  • rabernat 1
  • jhamman 1
  • keewis 1

issue 1

  • Html repr · 27 ✖

author_association 1

  • MEMBER · 27 ✖
id html_url issue_url node_id user created_at updated_at ▲ author_association body reactions performed_via_github_app issue
546229024 https://github.com/pydata/xarray/pull/3425#issuecomment-546229024 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjIyOTAyNA== benbovy 4160723 2019-10-25T07:00:26Z 2019-10-25T07:00:26Z MEMBER

These are handled in the same way as long variable names, they truncate with an ellipsis and then expand on hover.

Oh yes I missed it.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
546076945 https://github.com/pydata/xarray/pull/3425#issuecomment-546076945 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjA3Njk0NQ== choldgraf 1839645 2019-10-24T19:54:44Z 2019-10-24T21:08:19Z MEMBER

Actually, no that's totally out of date now haha. Thanks for reminding me I should update that

edit: realized I should have said what I'm actually using - I'm using this:https://jupyterbook.org/features/page.html

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
546084523 https://github.com/pydata/xarray/pull/3425#issuecomment-546084523 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjA4NDUyMw== shoyer 1217238 2019-10-24T20:16:13Z 2019-10-24T20:16:13Z MEMBER

To verify proper escaping, we could validate the generated HTML as XML: https://stackoverflow.com/questions/13742538/how-to-validate-xml-using-python-without-third-party-libs

This could be a good use case for Hypothesis. Or you could manually build a dataset with variables of every dtype.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
546072495 https://github.com/pydata/xarray/pull/3425#issuecomment-546072495 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjA3MjQ5NQ== benbovy 4160723 2019-10-24T19:42:09Z 2019-10-24T19:42:09Z MEMBER

Cool!

I guess this post about how you use notebooks in your blog is still up to date?

I looks like we'll need to fix a couple of issues (the channel coordinate looks weird), but maybe it is not related to the front-end.

Besides variable names, we probably need to somehow handle overflow for variables that have a long list of dimension labels.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
546062969 https://github.com/pydata/xarray/pull/3425#issuecomment-546062969 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjA2Mjk2OQ== choldgraf 1839645 2019-10-24T19:16:27Z 2019-10-24T19:17:03Z MEMBER

this is awesome! I was so excited about it that I updated my little blog post with the latest xarray master https://predictablynoisy.com/xarray-explore-ieeg

I was a little sad at the gigantic repr that came out of my DataArray, and now it is nice and tidy html!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
546053879 https://github.com/pydata/xarray/pull/3425#issuecomment-546053879 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjA1Mzg3OQ== mrocklin 306380 2019-10-24T18:52:08Z 2019-10-24T18:52:08Z MEMBER

Thanks @jsignell (and all). I'm really jazzed about this.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
546009495 https://github.com/pydata/xarray/pull/3425#issuecomment-546009495 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjAwOTQ5NQ== benbovy 4160723 2019-10-24T16:57:57Z 2019-10-24T16:57:57Z MEMBER

Thanks a lot @jsignell ! It is great to see this merged finally!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
546009437 https://github.com/pydata/xarray/pull/3425#issuecomment-546009437 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjAwOTQzNw== shoyer 1217238 2019-10-24T16:57:45Z 2019-10-24T16:57:45Z MEMBER

Should we expand the .data section of the DataArray HTML repr by default? That would look a little more consistent with the text repr.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
546006017 https://github.com/pydata/xarray/pull/3425#issuecomment-546006017 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NjAwNjAxNw== dcherian 2448579 2019-10-24T16:48:41Z 2019-10-24T16:48:41Z MEMBER

Awesome. Thanks @jsignell Let's leave dark theme compatibility for a future PR.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545997936 https://github.com/pydata/xarray/pull/3425#issuecomment-545997936 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTk5NzkzNg== benbovy 4160723 2019-10-24T16:28:22Z 2019-10-24T16:28:22Z MEMBER

Yes it looks nice!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545978412 https://github.com/pydata/xarray/pull/3425#issuecomment-545978412 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTk3ODQxMg== benbovy 4160723 2019-10-24T15:42:02Z 2019-10-24T15:42:02Z MEMBER

Mmm this still looks weird on my side:

Which version of jupyterlab are you using?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545974184 https://github.com/pydata/xarray/pull/3425#issuecomment-545974184 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTk3NDE4NA== shoyer 1217238 2019-10-24T15:32:36Z 2019-10-24T15:32:36Z MEMBER

+1 for merging this turned off by default, with a plan to switch it on once we're tested it on more datasets and platforms

On Thu, Oct 24, 2019 at 8:27 AM Deepak Cherian notifications@github.com wrote:

@dcherian commented on this pull request.

In xarray/core/options.py https://github.com/pydata/xarray/pull/3425#discussion_r338641677:

@@ -98,6 +102,9 @@ class set_options:

   attrs, ``False`` to always discard them, or ``'default'`` to use original

   logic that attrs should only be kept in unambiguous circumstances.

   Default: ``'default'``.
    • display_style: display style to use in jupyter for xarray objects.
  • Default: classic. Other options are html.

⬇️ Suggested change

  • Default: classic. Other options are html.

  • Default: 'text'. Other options are 'html'.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/3425?email_source=notifications&email_token=AAJJFVQMQKGXPGUFEMZ3ZRTQQG5HNA5CNFSM4JDFFVJKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCJDVXPI#pullrequestreview-306666429, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJJFVWYZ27FZ76RZ422M4LQQG5HNANCNFSM4JDFFVJA .

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545970448 https://github.com/pydata/xarray/pull/3425#issuecomment-545970448 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTk3MDQ0OA== benbovy 4160723 2019-10-24T15:24:24Z 2019-10-24T15:24:24Z MEMBER

I'd would rather wait before turning on html display by default. I'd be fine doing so on the next major release if until then we make sure that the rendering will look reasonably good in most front-end situations.

For example, it will really need better integration with jupyterlab dark themes (see below) and would potentially cause issues elsewhere too (nbsphinx, etc.). But we should probably figure that out after this PR is merged.

I've noticed an issue with the alignment of attributes in jupyterlab (1.1.4) (Firefox 69.0.3):

It looks good in the classic notebook.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545963507 https://github.com/pydata/xarray/pull/3425#issuecomment-545963507 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTk2MzUwNw== jhamman 2443309 2019-10-24T15:08:55Z 2019-10-24T15:08:55Z MEMBER

I would be fine having this be on by default now. Or, we could keep it off until the next major release (0.15).

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545653224 https://github.com/pydata/xarray/pull/3425#issuecomment-545653224 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTY1MzIyNA== keewis 14808389 2019-10-23T21:57:00Z 2019-10-23T21:57:00Z MEMBER

nope, it still fails. However, that failure is definitely not related to this PR (it fails to install because the referenced PR in cftime was merged in the meantime).

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545579133 https://github.com/pydata/xarray/pull/3425#issuecomment-545579133 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTU3OTEzMw== benbovy 4160723 2019-10-23T18:35:23Z 2019-10-23T18:35:23Z MEMBER

Or maybe "text" and "html"?

Hopefully "html" will eventually become the classic display :)

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545542742 https://github.com/pydata/xarray/pull/3425#issuecomment-545542742 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTU0Mjc0Mg== shoyer 1217238 2019-10-23T17:06:59Z 2019-10-23T17:06:59Z MEMBER

It looks like the html_repr for rasm is 38kb .

How are you measuring this?

I see: ```

len(rasm.repr_html().encode('utf8')) 19624 ```

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545534925 https://github.com/pydata/xarray/pull/3425#issuecomment-545534925 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTUzNDkyNQ== shoyer 1217238 2019-10-23T16:47:28Z 2019-10-23T16:47:28Z MEMBER

Here's what we're currently looking at!

{
    "total_count": 2,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 2,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545219988 https://github.com/pydata/xarray/pull/3425#issuecomment-545219988 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTIxOTk4OA== shoyer 1217238 2019-10-23T01:13:41Z 2019-10-23T01:13:41Z MEMBER

We probably should be careful to use html.escape() pervasively to protect against malformed data. We can trust user code, but probably should not trust all data -- it should be safe to load and view arbitrary netCDF files in xarray.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545205143 https://github.com/pydata/xarray/pull/3425#issuecomment-545205143 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTIwNTE0Mw== shoyer 1217238 2019-10-22T23:59:53Z 2019-10-23T00:02:37Z MEMBER

I just gave this a try in Chrome / Google Colab. It's looking great!

Here's the rasm dataset. It weighs in at exactly 20 KB!

The alignment of "Dimensions" line does looks a little off -- would that be easy to fix?

There's also a strange alignment thing with the three circle icon in array repr, which maybe could be moved to the right side?

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545200472 https://github.com/pydata/xarray/pull/3425#issuecomment-545200472 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTIwMDQ3Mg== shoyer 1217238 2019-10-22T23:37:46Z 2019-10-22T23:37:46Z MEMBER

The css is now ~300 lines and it gets injected everytime the dataset is rendered. I'm not sure if that is considered too long and if it is I don't quite know what to do about it.

You could consider running the CSS through some sort of preprocessing tool to minify it, e.g., to remove all the spaces. But my guess is that it's probably fine.

It would be informative to measure the size of the output HTML encoded as UTF-8 bytes for some example datasets. For context, a simple line chart from matplotlib is ~20 KB when saved as a PNG image. Anything in that ball-park would be OK, up to perhaps a few hundred KB for really big datasets with lots of variables.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
545113428 https://github.com/pydata/xarray/pull/3425#issuecomment-545113428 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NTExMzQyOA== shoyer 1217238 2019-10-22T19:19:20Z 2019-10-22T19:19:20Z MEMBER

Is there ever a situation in which dimensions would be expandable? It seems like it is hard-coded to collapsed.

I don't think it would ever be expanded. The collapsed icon is just there for visual consistency.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
544840593 https://github.com/pydata/xarray/pull/3425#issuecomment-544840593 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NDg0MDU5Mw== benbovy 4160723 2019-10-22T07:35:37Z 2019-10-22T08:17:21Z MEMBER

Just one small issue in the gist with the vertical alignment within a variable row, and the height of the icon container (I'm using Firefox):

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
544839504 https://github.com/pydata/xarray/pull/3425#issuecomment-544839504 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NDgzOTUwNA== benbovy 4160723 2019-10-22T07:31:46Z 2019-10-22T07:31:46Z MEMBER

This is really nice @jsignell !

I'd really like to hear back from @benbovy about whether the fiddle css is more uptodate.

We did a lot of experiments, even some after starting the implementation in #1820, so I admit this is quite messy. I also did some clean-up and tweaks directly in that PR, so unfortunately there is no fiddle of reference that exactly corresponds to what's in here. Sorry for that!

The html/css in this PR is the cleanest one, I think.

The latest fiddles (for Dataset) are these two ones:

  • https://jsfiddle.net/Lmqq7yzz/9/
  • https://jsfiddle.net/1g04ykum/ (@shoyer's try for handling long variable names)

the only thing that looks funny to me is the alternating black-gray text coloring

In the HTML repr, the width of the space character is less large that for the monospace font used in the text repr, so I originally used this trick to better distinguish between the values in the inline data repr. Now that each value is encapsulated in it's own span, we could tweak the padding instead (or both... I don't have strong opinion on this).

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
544770938 https://github.com/pydata/xarray/pull/3425#issuecomment-544770938 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NDc3MDkzOA== shoyer 1217238 2019-10-22T01:15:19Z 2019-10-22T01:15:19Z MEMBER

This looks fantastic! Really great to have you working on this!

Some specific feedback:

  1. Yes, I agree it would be nice to align the Attributes text, maybe this could make use of an HTML table?
  2. Maybe bold would look better than underline for coordinates that are dimensions? I find the underline makes text a little hard to read. Happy to leave this up to you, though.
  3. On my Ubuntu desktop, the "+" icon is bright yellow, which I'm guessing is not what was intended. Maybe it would better to stick with the three circle "data" icon?
  4. What happens if there is a really long variable name? I had a version of the jsfiddle that expanded names when you hovered over them.
{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
544769183 https://github.com/pydata/xarray/pull/3425#issuecomment-544769183 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NDc2OTE4Mw== rabernat 1197350 2019-10-22T01:05:40Z 2019-10-22T01:05:40Z MEMBER

Julia, it's fantastic to see you working on this! Thanks so much!

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810
544740927 https://github.com/pydata/xarray/pull/3425#issuecomment-544740927 https://api.github.com/repos/pydata/xarray/issues/3425 MDEyOklzc3VlQ29tbWVudDU0NDc0MDkyNw== dcherian 2448579 2019-10-21T22:50:40Z 2019-10-21T22:50:40Z MEMBER

Honestly this is looking super good and ready to merge already!

Thanks @jsignell

The only thing that looks funny to me is the alternating black-gray text coloring below:

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  Html repr 510294810

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]);
Powered by Datasette · Queries took 16.412ms · About: xarray-datasette