Bump ipytree from 0.2.1 to 0.2.2

[dependabot]

Bumps ipytree from 0.2.1 to 0.2.2.

Commits

• See full diff in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

---

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR
• @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
• @dependabot merge will merge this PR after your CI passes on it
• @dependabot squash and merge will squash and merge this PR after your CI passes on it
• @dependabot cancel merge will cancel a previously requested merge and block automerging
• @dependabot reopen will reopen this PR if it is closed
• @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
• @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

[dependabot]

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version. You can also ignore all major, minor, or patch releases for a dependency by adding an ignore condition with the desired update_types to your config file.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

[joshmoore]

@dependabot recreate

[joshmoore]

Change in API??

    def _ipython_display_(self):
        tree = tree_widget(self.group, expand=self.expand, level=self.level)
>       tree._ipython_display_()
E       AttributeError: 'Tree' object has no attribute '_ipython_display_'

[joshmoore]

Perhaps more an issue with the recursive nature of the function...?

[jakirkham]

Yeah not sure. It could also be a change in ipywidgets.

An ipytree.Tree instance is returned by tree_widget. So the issue is ipytree.Tree use to have _ipython_display_, but now doesn't. Not entirely sure why that is. Issue ( jupyter-widgets/ipywidgets#3552 ) looks vaguely similar.

cc @martinRenou (in case you have any insights here 🙂)

[codecov]

Codecov Report

Merging #1112 (786f732) into main (be0bf3e) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1112   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files          36       36           
  Lines       14142    14141    -1     
=======================================
- Hits        14135    14134    -1     
  Misses          7        7           

Impacted Files	Coverage Δ
setup.py	0.00% <ø> (ø)
zarr/tests/test_hierarchy.py	100.00% <100.00%> (ø)
zarr/util.py	100.00% <100.00%> (ø)

[jakirkham]

Think I figured out the issue, upstream (ipywidgets, upstream dependency of ipytree) switched to _repr_mimebundle_ instead of _ipython_display_. Since ipytree relaxed its upper bound on ipywidgets, that issue leaks through here.

To address this, made the same switch in Zarr.

AFAICT it is not really possible to support both. So just made this a breaking change and bumped the minimum versions of both ipytree and ipywidgets. Users wouldn't call these methods themselves (only seeing the pretty display that shows up). So really they are most interested in things behaving ok, which should be the case with these changes (CI to confirm 🍀).

[jakirkham]

There were a few other tweaks needed to get this to work locally (pushed), but this now seems to behave correctly 🙂

[martinRenou]

Indeed _ipython_display_ is being deprecated slowly and replaced by _repr_mimebundle_ (which IPython's display logic will call).

AFAICT it is not really possible to support both

You can probably support both by calling display(tree) instead of tree._ipython_display_() in your _ipython_display_ method (using from IPython.display import display)

[jakirkham]

Thanks Martin! 🙏

---

Indeed _ipython_display_ is being deprecated slowly and replaced by _repr_mimebundle_ (which IPython's display logic will call).

Ok so it sounds like moving to _repr_mimebundle_ would be desirable in that case.

---

You can probably support both by calling display(tree) instead of tree._ipython_display_() in your _ipython_display_ method (using from IPython.display import display)

Interesting, this sentence in the _ipython_display_ docs was what gave me pause.

If this is defined, all other display methods are ignored.

IIUC that means _repr_mimebundle_ wouldn't be used if _ipython_display_ is defined, which would mean the legacy method is still used. Is that right?

[joshmoore]

Whew. Thanks, both!

[jakirkham]

Oops sorry didn't mean to merge myself 🤦‍♂️

Trying to get use to a new computer so probably messed something up 😅 Happy to follow up more as needed

[joshmoore]

If things stay green, 👍

[martinRenou]

Sorry my comment about deprecating _ipython_display_ was wrong, I thought it was the case but it's not.

Though ipywidgets is not using _ipython_display_ anymore but _repr_mimebundle_, see this PR.

According to the IPython docs, I'm not sure supporting both makes sense then. But I believe my comment about calling display(tree) in your _ipython_display_ method still makes sense:

calling display(tree) instead of tree._ipython_display_() in your _ipython_display_ method (using from IPython.display import display)

And you probably don't need to implement _repr_mimebundle_.

[jakirkham]

Interesting. This would mean adding ipython as a requirement then, right?

[martinRenou]

It is already an ipywidgets dependency. Though it is being discussed to remove the ipython and ipykernel dependencies. I'd really like that to happen at some point, because we don't want to enforce ipywidgets to run inside ipykernel.

If you don't want to add an IPython dependency in zarr-python, then probably your PR here removing _ipython_display_ was good :) Because it makes your code less bound to IPython.