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

Quarto does not fail rendering when errors happen in IPython display step #12228

Open
cderv opened this issue Mar 7, 2025 · 1 comment · May be fixed by #12238
Open

Quarto does not fail rendering when errors happen in IPython display step #12228

cderv opened this issue Mar 7, 2025 · 1 comment · May be fixed by #12238
Assignees
Labels
bug Something isn't working jupyter

Comments

@cderv
Copy link
Collaborator

cderv commented Mar 7, 2025

This is from an issue discussion about a great_tables example

I managed to reduce to a minimal example but

here is the original example
---
format: html
---

```{python}
from great_tables import GT, nanoplot_options
import polars as pl
weather_2 = pl.DataFrame(
    {
        "station": ["Station " + str(x) for x in range(1, 4)],
        "temperatures": [
            {
                "x": [6.1, 8.0, 10.1, 10.5, 11.2, 12.4, 13.1, 15.3],
                "y": [24.2, 28.2, 30.2, 30.5, 30.5, 33.1, 33.5, 32.7],
            },
            {
                "x": [7.1, 8.2, 10.3, 10.75, 11.25, 12.5, 13.5, 14.2],
                "y": [18.2, 18.1, 20.3, 20.5, 21.4, 21.9, 23.1, 23.3],
            },
            {
                "x": [6.3, 7.1, 10.3, 11.0, 12.07, 13.1, 15.12, 16.42],
                "y": [15.2, 17.77, 21.42, 21.63, 25.23, 26.84, 27.2, 27.44],
            },
        ]
    }
)

(
    GT(weather_2)
    .fmt_nanoplot(
        columns="temperatures",
        plot_type="line",
        expand_x=[5, 16],
        expand_y=[10, 40],
        options=nanoplot_options(
            show_data_area=False,
            show_data_line=False
        )
    )
)
```

Here is the reproducible example

---
title: test
format: html
---

```{python}
# First cell - create an object with a buggy _repr_html_ method
class BuggyDisplay:
    def __init__(self):
        self.data = "This works fine"
    
    def _repr_html_(self):
        # This error happens during display, not execution
        raise ValueError("Display phase error!")
    
    def __repr__(self):
        # This ensures the object has a string representation
        return self.data

# Create the object
buggy = BuggyDisplay()
```

```{python}
# Second cell - display the object (triggers error in _repr_html_)
# This will show an error in the output but won't stop execution
buggy
```

```{python}
# Third cell - proves execution continues
print("Execution continued despite display error")
```

Image

The problem is that nbconvert (thus nbclient) is not throwing exception for error that happens in IPython step

quarto convert .\bug.qmd
jupyter nbconvert --to notebook --execute bug.ipynb

This does not fail. This means Quarto does not catch exception, and it behaves always as if error = true has been set.

Still unsure if this is expected behavior in nbconvert and Jupyter.

If we want to Quarto to fail, we can catch "output_type": "error" in output when no failure happened, and throw an exception from there possibly.

This needs a bit more investigation. Opening to track this happens.

@cderv cderv added bug Something isn't working jupyter labels Mar 7, 2025
@cderv cderv self-assigned this Mar 7, 2025
@cderv
Copy link
Collaborator Author

cderv commented Mar 7, 2025

Still unsure if this is expected behavior in nbconvert and Jupyter.

From further reading, I think nbconvert does not fail on display error because it is not error in the code itself, and it does not prevent computing an output from the exectuted notebook because there are fallback representation.

Here the error is in the way cell are displayed. Tool using nbclient or nbconvert can decide what to do with those errors. This also allow to have fallback representation in output.

This is because all representations are computed. From my previous example, first cell can be changed to

class BuggyDisplay:
    def __init__(self):
        self.data = "This works fine"
    
    def _repr_html_(self):
        # This error happens during display, not execution
        raise ValueError("Display phase error!")

    def _repr_markdown_(self):
        # Markdown representation as fallback when HTML fails
        return "**Markdown fallback:** " + self.data
    
    def __repr__(self):
        # This ensures the object has a string representation
        return self.data

# Create the object
buggy = BuggyDisplay()

The error will be shown in output but the cell output quarto uses is now the markdown fallback.

Image

If we inverse and make the markdown fail then the HTML is correctly used in output

class BuggyDisplay:
    def __init__(self):
        self.data = "This works fine"
    
    def _repr_html_(self):
        # This error happens during display, not execution
        # raise ValueError("Display phase error!")
        return "<b>HTML fallback:</b> " + self.data

    def _repr_markdown_(self):
        # Markdown representation as fallback when HTML fails
        # return "**Markdown fallback:** " + self.data
        raise ValueError("Display phase error!")
    
    def __repr__(self):
        # This ensures the object has a string representation
        return self.data

# Create the object
buggy = BuggyDisplay()

Image

but we still have an error shown in the ipynb output, and Quarto does show it.

So, nbconvert behavior is expected as the code does not error, but one of the representations does, which does not prevent having a "valid" output from their perspective.

In quarto, we need to decide what to do

  • Fail rendering when one of the displays fails?
  • Hide the error, as we already have a fallback to choose available representation anyway. In that case, we could pass the error as a warning in the console log (not in the output)
  • Other?

I have a fix for first so that we can discuss

@cderv cderv added needs-discussion Issues that require a team-wide discussion before proceeding further and removed needs-discussion Issues that require a team-wide discussion before proceeding further labels Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jupyter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant