issue_comments

I think @tomwhite has implemented this functionality. Thanks everyone.

I haven't implemented the nested options for display_expand - is it worth adding the extra complexity here?

Not sure :). Can you open a PR please?

EDIT: Oops you've already opened one! thanks!

Thanks for the suggestions @dcherian!

I've implemented "default" to preserve existing behaviour (i.e. only collapse if greater than preconfigured size limits). See #5126

I haven't implemented the nested options for display_expand - is it worth adding the extra complexity here?

maintaining the existing behaviour, which is that for the HTML dataset repr all three sections are expanded by default,

The existing behaviour could be "default", True could be fully expanded always; and False could be never expanded.

display_expand_attrs, display_expand_coords, display_expand_data, and display_expand_data_vars f

These are a lot with potentially two more: indexes and encoding.

Should we instead do something like python xr.set_options(display_expand = { "data_vars": False, "coords": "default", "attrs": False, })

I would find it useful to have a generalized version of this for datasets, so it's possible to control independently whether the coordinates, data variables, and attributes sections are expanded by default.

I'd be happy to pick this up if no one else is working on it. I have created a branch to try it out, which adds the options display_expand_attrs, display_expand_coords, display_expand_data, and display_expand_data_vars for controlling both HTML and text representations. All default to True.

One thing I'm not sure about is maintaining the existing behaviour, which is that for the HTML dataset repr all three sections are expanded by default, up to a certain size limit, depending on the section (25 coordinates, 15 data variables, 10 attributes). I couldn't see a simple way to incorporate that, although maybe it's not needed if the sections can be collapsed by setting an option (e.g. xr.set_options(display_expand_attrs=False))?

Hello @michaelaye! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-02-27 20:54:05 UTC

but I think a name like html_expand_data or html_collapse_data would be more descriptive, because this refer specifically to the "data" section of HTML reprs.

Do we want that this expand/collapse option affects the text repr too (i.e., collapsed = show inline values repr vs. expanded = show the whole data repr)? This might be more consistent, although obviously there's no way to interactively expand/collapse the data section in the text repr.

In that case display_expand_data or display_collapse_data would be better names (using the same prefix than the display_style option).

The scope is limited enough here that it would be fine to include the cleanup in this PR. The bigger issue is that names exposed in a public API need to be chosen much more carefully than internal only names — and we definitely don’t want to expose the current poorly chosen internal name for this feature to users.

On Sat, Aug 1, 2020 at 7:32 PM Michael Aye notifications@github.com wrote:

i'm fine in fixing the name of the variable, but usually open source teams don't want a PR that addresses more than one issue at a time, are you sure you want it in here?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/4230#issuecomment-667615325, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJJFVTTULSMPGZWCUNIGPTR6TF4LANCNFSM4O3LS7JA .

i'm fine in fixing the name of the variable, but usually open source teams don't want a PR that addresses more than one issue at a time, are you sure you want it in here?

I support adding an option for this, but I think a name like html_expand_data or html_collapse_data would be more descriptive, because this refer specifically to the "data" section of HTML reprs.

I also agree with @jsignell that the current naming seems to be reversed "expanded" should mean show the data section and "collapsed" should mean hiding it.

I'm looking at the relevant test function:

python def test_repr_of_dataarray(dataarray): formatted = fh.array_repr(dataarray) assert "dim_0" in formatted # has an expanded data section assert formatted.count("class='xr-array-in' type='checkbox' checked>") == 1 # coords and attrs don't have an items so they'll be be disabled and collapsed assert ( formatted.count("class='xr-section-summary-in' type='checkbox' disabled >") == 2 )

does this only check on the number of collapsed sections?

Yes, that test counts expanded data sections and collapsed summary sections

How could I check specifically if the data section is collapsed based on the OPTION setting?

Use set_option as a context manager and do a test much like the one above.

I'm looking at the relevant test function:

python def test_repr_of_dataarray(dataarray): formatted = fh.array_repr(dataarray) assert "dim_0" in formatted # has an expanded data section assert formatted.count("class='xr-array-in' type='checkbox' checked>") == 1 # coords and attrs don't have an items so they'll be be disabled and collapsed assert ( formatted.count("class='xr-section-summary-in' type='checkbox' disabled >") == 2 )

I'm not very good with HTML, does this only check on the number of collapsed sections? I am kinda guessing that this code is abusing checkbox code to make the display collapsible? How could I check specifically if the data section is collapsed based on the OPTION setting?

Oh, implemented the wrong default, things were confusing as the HTML code "checked" for the collapsed variable apparently means NOT collapsed...

For mypy I'm getting:

bash . is not a valid Python package name

I didn't isort this commit, usually other peeps hate it when i do it, happy that you actually require it! :)

But I'm getting this return while using isort -rc . in the root repo folder (is that the idea?)

bash $ isort -rc . (xarray-tests) /home/maye/miniconda3/envs/xarray-tests/lib/python3.7/site-packages/setuptools/distutils_patch.py:26: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first. "Distutils was imported before Setuptools. This usage is discouraged " Skipped 4 files