Migrate formatting_html.py into xarray core #8930
Conversation
oopsies This reverts commit d68ae3c.
|
Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient. |
|
Bringing @TomNicholas comment here!
Yeah, I can take a look at this. |
|
@TomNicholas I think I found the fix! I responded in the thread xarray-contrib/datatree#91 and updated in my latest commit. I am a little concerned that the |
xarray/static/css/style.css
Outdated
| @@ -28,7 +28,7 @@ body.vscode-dark { | |||
|
|
|||
| .xr-wrap { | |||
| display: block !important; | |||
| min-width: 300px; | |||
| min-width: 700px; | |||
There was a problem hiding this comment.
Updating the min-width to be 700px seems to solve this bug, however, now it is the same as the max-width. I am not sure if this is okay but thought I would point it out.
There was a problem hiding this comment.
Awesome! Are there other similar values to compare to in that file? Should we just have min-width=700px and max-width even bigger, or is the bug from them being different values somehow?
There was a problem hiding this comment.
So when i set max-width to 900px it doesn't make a difference for the grouped dataset (ompixcor) but it does increase the width for the non-grouped dataset (tree_hoss). I think it is something to do with the margin lines that represent the group hierarchies.
Here is an example:
Let me know what you think and I can update the max-width accordingly.
There was a problem hiding this comment.
That width controls what the repr looks like when the area that it has available is small. It basically says the width of the repr cannot be less than 700px. This mostly comes up in jupyterlab when you have a split view. Here's the difference:
main:
this PR:
see how the repr goes off screen? There is a horizontal scroll, so you can still get to the far right of the repr, but it's just a little less beautiful.
It seems like this change is related to xarray-contrib/datatree#91. Is there a reproducible example that I can mess around with?
There was a problem hiding this comment.
...and I broke some tests. Will fix those now.
There was a problem hiding this comment.
Fixed: fdf53f
It looks resolved to me, but want to check sure if @eni-awowale is okay with things as she saw different behaviour.
There was a problem hiding this comment.
I will check this out as soon as I can!
There was a problem hiding this comment.
Just tried it out and it's fixed! I will update this to not be a draft
There was a problem hiding this comment.
@TomNicholas - I think we're good to go.
xarray/core/formatting_html.py
Outdated
| @@ -341,3 +343,130 @@ def dataset_repr(ds) -> str: | |||
| ] | |||
|
|
|||
| return _obj_repr(ds, header_components, sections) | |||
|
|
|||
|
|
|||
| def summarize_datatree_children(children: Mapping[str, Any]) -> str: | |||
There was a problem hiding this comment.
Is this Any really a DataTree? (Not 100% sure, but it seems like a good guess)
There was a problem hiding this comment.
Thanks for fixing up the type hints!
|
@owenlittlejohns I noticed I am getting some circular import errors from importing |
Shouldn't that be solvable with |
Ahh yes that solves it! Thanks! |
|
Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again! |
This PR migrates the
formatting_html.pymodule intoxarray/core/formatting_html.pyas part of the on-going effort to mergexarray-datatreeintoxarray.One thing of note is that importing and setting the
OPTIONSto"default"indatatree/formatting_html.py(lines) were moved intoxarray/core/options.pyon #L23 and #L49. So, I did not add them back toxarray/core/formatting_html.py.datatree/formating_htmls.pyTrack merging datatree into xarray #8572User visible changes (including notable bug fixes) are documented in whats-new.rstNew functions/methods are listed in api.rst