Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ux): get rid of duplicated tracebacks #10002

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Sep 3, 2024

Based on #9811, I removed the code (_repr_mimebundle_) that was causing duplicated tracebacks. Instead I implemented _repr_pretty_ (for the consoles) and _repr_html_ for the notebook.

Closes #9990.

@cpcloud cpcloud added this to the 9.4 milestone Sep 3, 2024
@cpcloud cpcloud added the ux User experience related issues label Sep 3, 2024
ibis/expr/types/core.py Outdated Show resolved Hide resolved
@cpcloud cpcloud requested a review from jcrist September 3, 2024 13:23
@cpcloud
Copy link
Member Author

cpcloud commented Sep 3, 2024

This fixes the triplicate expression repring in the console and reduces the number of tracebacks rendered by one in the notebook (via quarto), so there's still two there. I'm going to poke at it in jupyterlab to see if it's a quarto rendering issue, or it's something in jupyter.

ibis/expr/types/core.py Outdated Show resolved Hide resolved
@cpcloud
Copy link
Member Author

cpcloud commented Sep 3, 2024

There is no solution something that fixes the issues we see and reprs tables aesthetically the same.

Here's the issue:

When both _repr_html_ and either __repr__ or _repr_pretty_ are defined, both _repr_html_ and the other (but of course not both!) are called. In our case, since they're both sharing the same rich console rendering code, they both raise the same exception which gets printed to the notebook and then raised, so it shows up twice.

Possible solutions

Define only _repr_html_

If only _repr_html_ is defined, then it's not possible to invoke __repr__ with an exception (and not even possible to call _repr_pretty_ at all), so the exception appears once. Since _repr_html_ only works in the notebook, this solution is a non-starter.

Define only _repr_pretty_

If only _repr_pretty_ is defined, then apparently __repr__ isn't called (contrary to what's said in ipython/ipython#9771), or least only a single traceback shows up.

The output of that however isn't nearly as nice as with _repr_html_.

Define only __repr__

This is the same as defining only _repr_pretty_.

This hack that might work: None-ing a failing _repr_html_ into oblivion 🕳️

Reading the docs I saw this:

image

I tried catching the exception only in _repr_html_, and that seems to do the trick:

  1. __repr__ is unaffected
  2. _repr_pretty_ is unaffected
  3. _repr_html_ only renders the traceback once
  4. all the things remain aesthetically the same

Ergo, hacking provides for nice things (??)

@cpcloud
Copy link
Member Author

cpcloud commented Sep 3, 2024

Kinda small, but you can the traceback shows up only once here:

image

@cpcloud
Copy link
Member Author

cpcloud commented Sep 3, 2024

I can also bring back _repr_mimebundle_ now I believe.

@cpcloud cpcloud force-pushed the dedup-tracebacks branch 5 times, most recently from cfa6be9 to 9ab3590 Compare September 3, 2024 15:14
@cpcloud
Copy link
Member Author

cpcloud commented Sep 3, 2024

Naturally, something is cursed here.

@cpcloud
Copy link
Member Author

cpcloud commented Sep 3, 2024

Not sure what was happening but something in the commit from #9811 was causing one of the quarto notebook (imdb.qmd) to hang forever.

I removed those changes entirely, and just committed the ones that fixes the double/triple exception rendering.

@cpcloud
Copy link
Member Author

cpcloud commented Sep 3, 2024

I have something that can also improve the exception look on that docs page, will do in a follow-up.

@cpcloud
Copy link
Member Author

cpcloud commented Sep 3, 2024

Just to wrap this up, here's the current state of the art for the various interpreters/notebooks:

Code

from ibis.interactive import *

t = ibis.memtable({"x": [1, 2, 3]})
t2 = ex.penguins.fetch()

t2.island + t.x.cast(str)  # expected exception

Vanilla Python REPL

>>> from ibis.interactive import *
>>>
>>> t = ibis.memtable({"x": [1, 2, 3]})
>>> t2 = ex.penguins.fetch()
>>>
>>> t2.island + t.x.cast(str)  # expected exception
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/cloud/src/github.com/ibis/ibis/expr/types/core.py", line 83, in __repr__
    return _capture_rich_renderable(self)
  File "/home/cloud/src/github.com/ibis/ibis/expr/types/core.py", line 63, in _capture_rich_renderable
    console.print(renderable)
  File "/nix/store/kzjp79qwj05yn1jh08d7j74szsfkq08m-python3-3.10.14-env/lib/python3.10/site-packages/rich/console.py", line 1710, in print
    extend(render(renderable, render_options))
  File "/nix/store/kzjp79qwj05yn1jh08d7j74szsfkq08m-python3-3.10.14-env/lib/python3.10/site-packages/rich/console.py", line 1311, in render
    render_iterable = renderable.__rich_console__(self, _options)  # type: ignore[union-attr]
  File "/home/cloud/src/github.com/ibis/ibis/expr/types/core.py", line 106, in __rich_console__
    rich_object = to_rich(self, console_width=console_width)
  File "/home/cloud/src/github.com/ibis/ibis/expr/types/pretty.py", line 274, in to_rich
    return _to_rich_table(
  File "/home/cloud/src/github.com/ibis/ibis/expr/types/pretty.py", line 318, in _to_rich_table
    table = tablish.as_table()
  File "/home/cloud/src/github.com/ibis/ibis/expr/types/generic.py", line 1538, in as_table
    raise com.RelationError(
ibis.common.exceptions.RelationError: Cannot convert <class 'ibis.expr.types.strings.StringColumn'> expression involving multiple base table references to a projection

IPython REPL

image

Jupyter notebook

image

Quarto page

image

vscode

image

@cpcloud cpcloud changed the title fix(ux): gut repr code; get rid of duplicated tracebacks fix(ux): get rid of duplicated tracebacks Sep 3, 2024
@cpcloud cpcloud merged commit 7df4bdd into ibis-project:main Sep 3, 2024
81 checks passed
@cpcloud cpcloud deleted the dedup-tracebacks branch September 3, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ux User experience related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs: Exception stack traces on the "Datatypes and Datashapes" page
2 participants