Skip to content

Express string output are quoted and wrapped in a code block #872

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

Closed
cpsievert opened this issue Dec 7, 2023 · 3 comments · Fixed by #905
Closed

Express string output are quoted and wrapped in a code block #872

cpsievert opened this issue Dec 7, 2023 · 3 comments · Fixed by #905
Labels

Comments

@cpsievert
Copy link
Collaborator

I find this behavior surprising (both the quoting and wrapping of strings into a code block):

import shiny.express

"Hello"
"express"
Screenshot 2023-12-07 at 1 22 24 PM

I would have expected a result closer to:

from shiny import ui
import shiny.express

ui.div("Hello")
ui.div("express")
Screenshot 2023-12-07 at 1 24 55 PM

Especially considering it's not terribly hard to opt-into code blocks (and I think we've been considering adding a more official one)?

from shiny import ui
import shiny.express

ui.markdown(
    """
```
Hello
```

```
express
```
"""
)

But maybe there's additional rationale for this as a default that I'm not aware of?

@wch
Copy link
Collaborator

wch commented Dec 7, 2023

This is the line where plain strings get wrapped into a pre tag:

self.args.append(tags.pre(repr(value)))

I was ambivalent about going this way vs rendering it as a plain string. I ended up going with pre because it's similar to what a Jupyter notebook would do, so the behavior for strings would be similar between Quarto-Shiny, Jupyter-Shiny, and Shiny Express.

I don't feel strongly committed to that direction, though. But if we change it, we will have to do a bit of education about why things look different in those different environments. In Express, the default would be plain text; you would have to wrap in a ui.pre() to get preformatted text; and in Quarto and Jupyter, the default would be preformatted text, and you would have to wrap in ui.div() to get plain text.

@gadenbuie
Copy link
Collaborator

This behavior was also surprising to me too. Without the additional context of how things work in notebook contexts, it feels out of place that strings are wrapped in <pre> tags. In notebooks, users have bit more control over how the results are written. For example, they might set output: asis for a code cell or a globally for their document, and managing the output is more expected in the environment.

Relatedly, it'd be nice if Quarto supported output: asis for strings without having to also call print(). If Shiny Express in a .py file treated strings as bare text nodes, this would bring the two formats in line.

Quarto Python Dashboard Example

Here's an example with Quarto. Notice the cell option #| output: asis combined with print(). It'd be nice if only output: asis were required, but the same happens in R as well (you need to call cat() on the string).

---
title: "Penguin Bills"
format: dashboard
server: shiny
---

```{python}
import seaborn as sns
penguins = sns.load_dataset("penguins")
```

## {.sidebar}

```{python}
from shiny import render, ui
ui.input_select("x", "Variable:",
                choices=["bill_length_mm", "bill_depth_mm"])
ui.input_select("dist", "Distribution:", choices=["hist", "kde"])
ui.input_checkbox("rug", "Show rug marks", value = False)
```

## Column

```{python}
#| output: asis
print(f"There are {len(penguins)} penguins in this dataset.")
```

```{python}
@render.plot
def displot():
    sns.displot(
        data=penguins, hue="species", multiple="stack",
        x=input.x(), rug=input.rug(), kind=input.dist())
```

image

A middle ground might be to treat top-level and child strings differently.

from shiny.express import layout

"Hello"
with layout.div():
	"express"

If the outer "Hello" were wrapped in a <pre> tag, then it would be consistent across all environments. But wrapping the "express" inside the div() in a <pre> tag is probably not what the user wants. Also, in the current case, it's odd that shiny.express.layout.span() and shiny.ui.span() give such different results

from shiny.express import layout
from shiny import ui

"Hello"
with layout.div():
    with layout.span():
    	"express"

with layout.div():
    ui.span("express")

image

@wch
Copy link
Collaborator

wch commented Dec 12, 2023

After a conversation with @cpsievert and @jcheng5, we settled on this:

  • Strings will not be wrapped in a <pre> tag by default. (They will be treated the same at the top level or inside of a with block.)
  • Users can use tags.pre() if they want it to display as a code block.
  • We will encourage users to use ui.markdown() if they want to easily write formatted text.

Other notes:

  • Now that htmltools allows using Tag functions as context managers (as in with ui.div():), it wouldn't make sense to wrap text inside of that in a <pre> tag, because you might want to do something like this:

    with ui.div():
        "hello world"

A summary of the reasoning:

Generally speaking, there are two kinds of text you'd want to put in a document:

  • Normal text, like if you are writing an article
  • The result of a computation

In Jupyter and Quarto, there is an easy way to add both types of text:

  • Normal text goes outside of code cells
  • Computational results go inside code cells

In the current form of Shiny Express, it works like this:

  • Normal text goes inside div()
  • Strings are displayed as computational results

In the proposed form of Shiny Express:

  • Strings are displayed as normal text
  • To display as computational results, wrap strings with ui.pre().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants