issue_comments

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.

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

+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 .

It looks like the html_repr for rasm is 38kb .

How are you measuring this?

I see: ```

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

Here's what we're currently looking at!

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.

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?

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.

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.

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.