Use ipytree in `.tree` representation by jakirkham · Pull Request #450 · zarr-developers/zarr-python

jakirkham · 2019-07-04T22:40:55Z

Reimplements the .tree representation in the Jupyter Notebook using ipytree, a Jupyter-widget wrapped jsTree implementation. This is similar to what we had before in that it still uses jsTree under-the-hood. However this is significantly easier for us to maintain and test. Not to mention it works seamlessly in both the legacy notebook and JupyterLab. This does require an extra installation step on the part of the user though. FWIW this was surprisingly quick to implement. 🙂

~~It would be good to reference how users should install ipytree. Also still needs a little work to get testing on CI going.~~ That said, this could be handy for a slide or a blogpost while we iron these things out.

TODO:

Add unit tests and/or doctests in docstrings
Add docstrings and API docs for any new/modified user-facing classes and functions
New/modified features documented in docs/tutorial.rst
Changes documented in docs/release.rst
Docs build locally (e.g., run tox -e docs)
AppVeyor and Travis CI passes
Test coverage is 100% (Coveralls passes)

For users wishing to have a nice representation of groups in Jupyter notebooks, `ipytree` is recommended to make these look a bit nicer as well as enable interactivity. Though it is not a requirement as we can always fallback to line representations. We make `ipytree` an optional dependency as there is a little more work to install it depending on the use case. So we want users to handle this intentionally.

alimanfoo · 2019-07-04T23:07:02Z

This certainly cleans things up a lot! Very nice.

jakirkham · 2019-07-04T23:10:31Z

Yeah it's also much easier to test. We can inspect nodes and see how they relate to each other. Haven't dug into that, but it is definitely an option. 🙂

For testing purposes, make sure that `_ipython_display_` returns the tree generated.

As we have replaced `_repr_html_` with `_display_ipython_`, drop tests related to it.

Make sure that the `ipytree` extension is installed correctly in the `tox` environment so the `.tree` representation can be tested.

These don't seem to work well with ipytree. So just drop them. Can always add them back if we or upstream figure out how to make them work.

jakirkham · 2019-07-05T00:25:41Z

Have pushed the notebook with this change for comparison. Can see with nbviewer.

Different icons don't work as well. So just dropped those examples.

alimanfoo · 2019-07-05T07:21:52Z

Looks very nice :-) I'm getting an odd behaviour viewing on my phone, tapping on a node makes it disappear. Then tapping another node makes the first node reappear, but the tapped node disappear. Maybe something to do with styling for selected node. Haven't checked on my laptop yet, don't know if specific to phone.

…

On Fri, 5 Jul 2019, 01:25 jakirkham, ***@***.***> wrote: Have pushed the notebook with this change for comparison. Can see with nbviewer <https://nbviewer.jupyter.org/github/zarr-developers/zarr-python/blob/e7a1a9a2bb1042cf2972965516467aaa32100eba/notebooks/repr_tree.ipynb> . Different icons don't work as well. So just dropped those examples. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#450?email_source=notifications&email_token=AAFLYQRS7CVDQZI6FM4P2ADP52IINA5CNFSM4H6A4QBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZIITQA#issuecomment-508594624>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFLYQXK7EM7JNKPHVD52I3P52IINANCNFSM4H6A4QBA> .

jakirkham · 2019-07-05T15:40:41Z

Thanks for the feedback. This seems to be caused by selecting the icons in nbviewer. Doesn't happen in the actual Jupyter notebook.

As the point of our tree display is not to select nodes, disable this feature. Hopefully works around a bug where icons disappear in nbviewer upon selection.

Now with all nodes disabled to avoid selection. Hopefully avoids the bug related to icons disappearing in nbviewer on selection.

jakirkham · 2019-07-05T15:50:44Z

Have pushed a change that disables all nodes so they cannot be selected. This seems to workaround the issue.

jakirkham · 2019-07-05T15:52:07Z

Here's an updated link.

alimanfoo · 2019-07-05T16:06:54Z

Cool, thanks John, looks good.

…

On Fri, 5 Jul 2019, 16:52 jakirkham, ***@***.***> wrote: Here's an updated link <https://nbviewer.jupyter.org/github/zarr-developers/zarr-python/blob/11cbdb1e262167a8905c0cea1d879a6ec706a007/notebooks/repr_tree.ipynb> . — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#450?email_source=notifications&email_token=AAFLYQW4BVZ4B4TUXWCEA63P55U2RA5CNFSM4H6A4QBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZJ27KI#issuecomment-508800937>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFLYQSCMATDKVB4BXRAYS3P55U2RANCNFSM4H6A4QBA> .

jakirkham · 2019-07-22T23:09:43Z

@alimanfoo, would you have a chance to do a code review?

alimanfoo · 2019-07-23T12:51:11Z

Hi @jakirkham, code looks good to my eye, however looking at the example notebook I don't think the expand argument is working correctly, because the trees with expand=True, expand=1, expand=2 and expand=3 all look the same.

Fixes an issue where `expand` did not work correctly in subtrees.

Should show the use of `expand` correctly.

jakirkham · 2020-01-04T02:10:31Z

Good catch @alimanfoo! Sorry for the long delay. As with most bugs this was a simple thing 😉

Have rerun on the notebook. Results can be seen on nbviewer.

jakirkham · 2020-01-07T15:29:44Z

Not sure what is up with Coveralls. Tried messing with it last night and this morning, but it still doesn't seem to be working. Otherwise CI passes.

jrbourbeau

The changes here looks good to me, thanks for refactoring @jakirkham! This seems a lot more maintainable now. Is there anything else you'd like to include, or is this good to merge?

jakirkham · 2020-01-09T04:50:16Z

Thanks for the review @jrbourbeau! Nothing more from me. Merge away 😀

jakirkham added 4 commits July 4, 2019 17:32

Move icon global variables up
cf073b4

Use icons that work well with ipytree
2e994a1

Add a utility function to get icon type
faaa2ed

jakirkham mentioned this pull request Jul 4, 2019
HTML jstree doesn't work in jupyter lab #259
Closed

jakirkham added 9 commits July 4, 2019 20:10

Revamp HTML representation to use ipytree
39e69ae

Make sure _ipython_display_ returns tree
dcdc202
For testing purposes, make sure that `_ipython_display_` returns the tree generated.

Test tree_get_icon utility function
e4fc404

Drop _repr_html_ tests
a5a7404
As we have replaced `_repr_html_` with `_display_ipython_`, drop tests related to it.

Test tree's notebook representation
0208d7b

Configure tox to test .tree representation
948302f
Make sure that the `ipytree` extension is installed correctly in the `tox` environment so the `.tree` representation can be tested.

Update .tree repr notebook
8cb0a8b

Note ipytree is needed for nice Jupyter rendering
fd21248

Drop changing icon .tree examples
e7a1a9a
These don't seem to work well with ipytree. So just drop them. Can always add them back if we or upstream figure out how to make them work.

jakirkham mentioned this pull request Jul 5, 2019
Alternative representations jupyter-widgets-contrib/ipytree#8
Closed

jakirkham added 2 commits July 5, 2019 11:44

Disable all nodes to disallow selection
5f5663f
As the point of our tree display is not to select nodes, disable this feature. Hopefully works around a bug where icons disappear in nbviewer upon selection.

Regenerate .tree notebook
11cbdb1
Now with all nodes disabled to avoid selection. Hopefully avoids the bug related to icons disappearing in nbviewer on selection.

jakirkham added 3 commits January 3, 2020 17:46

Merge zarr-developers/master into jakirkham/use_ipytree
0465243

Pass expand as a keyword argument
3b638ef
Fixes an issue where `expand` did not work correctly in subtrees.

Rerun repr_tree.ipynb and save
45b3ff2
Should show the use of `expand` correctly.

jakirkham mentioned this pull request Jan 6, 2020
v2.4 release #529
Closed

Merge remote-tracking branch 'upstream/master' into use_ipytree
f3d1c7d

jrbourbeau approved these changes Jan 9, 2020
View reviewed changes

jrbourbeau merged commit a9bcafd into zarr-developers:masterJan 9, 2020

jakirkham deleted the use_ipytree branch January 9, 2020 20:03

ryan-williams mentioned this pull request Feb 17, 2020
zarr.tree() example in docs doesn't work as of 2.4.0 #537
Closed

Carreau added this to the v2.4 milestone Sep 9, 2020

Uh oh!

Uh oh!

Conversation

jakirkham commented Jul 4, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alimanfoo commented Jul 4, 2019

Uh oh!

jakirkham commented Jul 4, 2019

Uh oh!

jakirkham commented Jul 5, 2019

Uh oh!

alimanfoo commented Jul 5, 2019 via email

Uh oh!

jakirkham commented Jul 5, 2019

Uh oh!

jakirkham commented Jul 5, 2019

Uh oh!

jakirkham commented Jul 5, 2019

Uh oh!

alimanfoo commented Jul 5, 2019 via email

Uh oh!

jakirkham commented Jul 22, 2019

Uh oh!

alimanfoo commented Jul 23, 2019

Uh oh!

jakirkham commented Jan 4, 2020

Uh oh!

jakirkham commented Jan 7, 2020

Uh oh!

jrbourbeau left a comment

Choose a reason for hiding this comment

Uh oh!

jakirkham commented Jan 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jakirkham commented Jul 4, 2019•
edited
Loading