Skip to content

Consider renaming shiny.express.layout #854

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 6, 2023 · 14 comments
Closed

Consider renaming shiny.express.layout #854

cpsievert opened this issue Dec 6, 2023 · 14 comments
Labels

Comments

@cpsievert
Copy link
Collaborator

cpsievert commented Dec 6, 2023

The name layout seems to be over-optimizing for with layout.sidebar(). It gets awkward with things like layout.layout_*(), layout.page_*(), etc.

I think I like ux as an alternative. Here are some pros/cons I can think of:

Pros

  • Short and similar to ui, which helps with the mental model of "use ux as an alternative of ui when you want to use @render.* decorators as children"
  • Serves as a more general home for UI things that might not be layout related.

Cons

  • It's maybe too similar to ui (hurts readability), but the fact that ux things will always start with ux will provide a clear enough distinction.
@gadenbuie
Copy link
Collaborator

I also like ux. Where UI refers to the visual elements and design of a website (the user interface), UX encompasses the overall experience of users while using a website (the user experience). These terms are also aligned with the difference between shiny.ui and shiny.express.ux: the Shiny Express version of UI isn't just an interface but can also include outputs. It's directionally aligned with the UX terminology and hints that Shiny Express elements can also wrap the outputs.

I also see a lot of value in being able to mix Express and core UI elements, so I see the closeness as useful to be able to switch between implementations if you realize you need one or the other.

from shiny import ui
from shiny.express import layout as ux

with ux.div():
  "something in a div"

ui.div("something also in a div")

@gshotwell
Copy link
Contributor

My sense of what's important for express is the following:

  1. Make Express succinct and easy to use
  2. Make it easy to move from Express to Classic
  3. Clearly differentiate Express and Classic

It seems that mixing Classic containers and Express containers is a bit of an edge case in that world, and so we shouldn't optimize for that use if it undermines the other goals.

I really don't like the ui/ux because it's so easy to mess up when copying and pasting, and it'll make our documentation very confusing. I would have trouble distinguishing these two blocks of code, so a new learner would probably have more trouble.

from shiny import ui
from shiny.express import layout as ux

with ui.div():
  "something in a div

ux.div("something also in a div")
from shiny import ui
from shiny.express import layout as ux

with ux.div():
  "something in a div"

ui.div("something also in a div")

I think if we want to bring the two interfaces together then it's probably better to have express.ui contain all of the functions we expect people to use in express, even if most of them are aliases to shiny.ui. This would also be beneficial because we would then be able to not include functions which don't work in express (like ui.output_*). Mixing layouts with that approach would be a bit more typing, but much less prone to error.

from shiny.ui import div as classic_div
from shiny.express import ui

with ui.div():
  "something in a div
  
classic_div("somethind also in a div")

@cpsievert
Copy link
Collaborator Author

cpsievert commented Dec 8, 2023

It seems that mixing Classic and Express containers is a bit of an edge case in that world, and so we shouldn't optimize for that use if it undermines the other goals.

As it currently stands, you can't really make anything useful without mixing the two, due in large part because it doesn't make a lot of sense to have "terminal" components (e.g., ui.input_*(), ui.card_header(), etc) be context managers.

Clearly differentiate Express and Classic

I can see the value in this when it comes to the coupling/decoupling of ui and server logic, but at least under the current implementation, there is an intentional and useful connection when it comes to UI components. If I where to teach the difference today, it would be "use shiny.express.ux over shiny.ui when you want server logic embedded inside the UI component(s)"

@gshotwell
Copy link
Contributor

Oh sorry I was unclear, I meant mixing container functions from shiny.ui with context managers from shiny.express

@wch
Copy link
Collaborator

wch commented Dec 9, 2023

I agree with @gshotwell's opinion that ui and ux are too similar and could be easily confused. I had previously advocated for ctx_ui or xui, which is more visually distinct. But now I actually don't think we should go that way, and am moving toward the idea that we should:

  • Rename express.layout to express.ui
  • Move more things into express.ui

Let's suppose we went with xui instead of `layout. That's mostly the same as the current state of things. You might write code like this:

from shiny import ui
from shiny.express import xui

with xui.sidebar():
    ui.input_action_button("go", ui.tags.b("Go!"))
    ui.div("Hello")
    with xui.div(style = "color: red;"):
        ui.span("World!")

Here are some of the good/bad/notable things:

  • Good: Very clear distinction of which things must be used with with, and which things must not be.
  • Good: "Ejection" to Shiny Prime is straightforward.
  • Bad: Two separate import statements, and users have to think about using functions from one module in some cases, and functions from another module in other cases.
  • Bad: There are functions in shiny.ui that users must not use, like ui.sidebar(), but it is not obvious what those functions are, and why it is that they can't be used. (In the case of ui.sidebar(), won't set the page function, whereas layout.sidebar() will do that.) So there is a set of functions in ui that are forbidden, but they can't find out about it until runtime.
  • Notable: The "World!" is wrapped inside of a ui.span(). This is because when an expression returns bare text, it is normally wrapped in a <pre> tag. That happens so that the behavior is consistent across Shiny Express, Quarto-Shiny and Jupyter-Shiny. See Express string output are quoted and wrapped in a code block #872 (comment) for more about this.

Now, here's another possibility that is similar to @gshotwell's proposal:

from shiny.express import ui

with ui.sidebar():
    ui.input_action_button("go", ui.tags.b("Go!"))
    ui.div("Hello", style = "color: blue;")
    with ui.div(style = "color: red;"):
        ui.span("World!")

In this case, there are three categories of functions, all in express.ui:

  • Functions that can only be used as context managers, like ui.sidebar()
  • Leaf node component functions that can only be used as non context managers, like ui.input_action_button()
  • Components that can be used both as context managers and as regular functions, like ui.div(). This is enabled by Make Tag objects usable as context managers py-htmltools#76.

Good/bad/notable things about this:

  • Good: Only one import. Users always call functions from that module.
  • Good: Don't have any forbidden functions.
  • Good: "Ejection" to Shiny Prime is straightforward.
  • Bad: In some dev environments, it's not obvious which functions can be used as context managers and which can't -- although pyright in VS Code and shinylive do flag this.
  • Notable: Some "leaf node" components, like input_action_button, can actually take other components as children.
  • Notable: The ui.span() wrapping is still necessary, as in the previous case.

@gshotwell
Copy link
Contributor

My sense of Streamlit and Gradio users is that they pretty much exclusively use context management for layout. Here's an example of a fairly complicated and well regarded Gradio app. My guess is that these users would probably not think to create a container using the functional form unless we specifically told them to do so so if we're optimizing for these folks then the downside is probably worth the simplicity of the single input.

I think if we exclusively use context management in our express examples and functional form in our prime examples then most users won't confuse the two. We can include some extended documentation which specifies which functions can be used in both forms for users who are interested in that.

@gadenbuie
Copy link
Collaborator

I have a couple questions, mostly out of curiosity and for my own edification:

  • With the move from express.layout to express.ui, will express.ui contain a mix of context managers and normal functions?
  • Can context manager functions also be called directly or must they always be called with with?
  • If the form is mutually exclusive, how will users know which functions require with?
  • What will happen if a user calls a context manager directly or an ordinary function with with?

@wch
Copy link
Collaborator

wch commented Dec 13, 2023

  • With the move from express.layout to express.ui, will express.ui contain a mix of context managers and normal functions?

Yes. An advantage of this is that everything is in the same module, and all functions from that module are usable in Express apps. In contrast, if there are two separate imports for ui and express.ui, then some of the functions from ui can't be used in express apps, but it's not obvious which.

The drawback of doing it this way is that it's not obvious which functions are context managers and which are not. That said, shinylive and pyright/VS Code will flag invalid uses in either direction.

  • Can context manager functions also be called directly or must they always be called with with?

There are some functions that must be used as context managers. There are others that can't be used as context managers. And there's a third category: functions that can be used either way. This category includes the Tag functions from htmltools.

It would be nice if all functions that could be used as context managers could also be used as normal functions, but when I tried to make this work earlier, I ran into some roadblocks that seemed insurmoutable. It seems like it's worth taking another look at this, though.

  • If the form is mutually exclusive, how will users know which functions require with?

The general heuristic is that nestable components use with. The heuristic isn't perfect -- for example, input_text() can take a label that is a Tag object (which is nestable), but it would not be used with with.

  • What will happen if a user calls a context manager directly or an ordinary function with with?

After #905, where results aren't automatically put in a pre block, an app with this:

# Use a context manager as a regular function
shiny.express.ui.value_box("title", "Value")

Will throw an error like this:

    raise TypeError(
TypeError: Invalid tag item type: <class 'shiny.express._recall_context.RecallContextManager'>. Consider calling str() on this value before treating it as a tag item.

On other side:

with shiny.ui.nav_spacer():
    "Content"

Results in this error:

TypeError: 'Nav' object does not support the context manager protocol

@gadenbuie
Copy link
Collaborator

Thanks for the detailed explanation, @wch! Is it technically possible for us to improve those error messages in the future? If we're able, it'd be nice to give clear, actionable advice about how to fix the "right function, wrong context" problem.

As a relative newcomer approaching Shiny Express, I have to say I've found it very helpful that where the function lives gives me a strong hint about how to use the function. I'd personally prefer to learn "functions from shiny.express.layout need to be used in this way" than have to learn the correct usage on an individual level.

@wch
Copy link
Collaborator

wch commented Dec 14, 2023

On one hand, I'm sympathetic to the idea of having a clear namespace distinction between context managers components and regular components. On the other hand, it's nice to have a single import, so the user doesn't have to think about which module to use each time they use one of the functions -- everything they'd want to use is in one place.

To help make it clear whether a function is a context manager or regular function, it would be good for the docstrings of context managers to clearly indicate that at the top.

I think we can improve the error messages when a context manager is used as a regular function. I don't think that we can do that for the reverse situation, where a regular function is used as a context manager (well, we could add __enter__ and __exit__ methods that alway throw, and while that would help at run time, it would remove the ability of a static type checker to detect the problem when someone does with regular_ui_fn():).

@gadenbuie
Copy link
Collaborator

gadenbuie commented Dec 14, 2023

I'm worried we're trading a small convenience -- a single import -- for many small inconveniences while writing and developing Shiny Express apps.

If we separate context manager components and functional components, then users may need to import both modules if they want to use a mix of both components. Managing imports seems to be a common and normal activity for anyone developing in Python. So in this scenario we'd need to teach people:

  1. shiny.express.layout are contexts managers, use them with with ____():
  2. shiny.ui are functional, use them like ____().
  3. Start creating a Shiny Express app with shiny.express.layout and augment with shiny.ui if or as needed.

If shiny.express.ui contains a mix of context managers and functional components, we can tell people to just from shiny.express import ui, but we also need to teach them:

  1. How to find out which components are context managers and which are functional.
  2. How to parse the error message when they use a context manager in a functional form.
  3. How to parse the error message when they use a functional component in the context manager form.
  4. How to import both modules anyway if they happen to want to use a component that is a context manager in shiny.express.ui in a functional form.

From an educational perspective, the first scenario is a much easier teaching task. It's a simpler mental model and it's all information you can provide up front when teaching Shiny Express. On the other hand, the second scenario is educationally complex. Someone trying Shiny Express for the first time will definitely end up using a component in the wrong form and will run into the error scenarios without having the full context to debug them. We could alleviate that friction with more details up front, but this will make Shiny Express feel more complicated to learn, so we'd have to strike a balance in our docs.

Above you mentioned there are some edge cases where shiny.ui components can't be used in Shiny Express. From the perspective of a new user, an edge case like shiny.ui.sidebar() represents a single edge case that's relatively easy to remember (esp. since there's already a shiny.express.layout.sidebar()). On the other hand, for new users, each component in shiny.express.ui that only supports the functional form is an individual edge case and the cost of learning is mostly equal since they won't yet have a mental model that can accommodate the nuance of the heuristics like "nestable components".

@jcheng5
Copy link
Collaborator

jcheng5 commented Dec 15, 2023

If the form is mutually exclusive, how will users know which functions require with?

I understand your concern here but I actually don't think it's going to be that confusing in practice. Our goal would be that the documentation for every context manager function would start with "Context manager" so you can immediately see it when the autocomplete/help popup appears. And for things that just take arguments (like input_button(label=...)) it will be obvious from the signature.

And also I think in practice the functions that we have fall pretty clearly into "container-ish" or "widget-ish". The only one that gives me pause is card_header, which sort of depends on whether you think it should be for card titles exclusively (widget/functional) or for potentially complicated stuff (container/context-manager).

The big exception to the above is HTML tags, because they will all support both. I don't have an intuition yet whether that will muddy the waters--I was thinking of not even explaining that there are two paradigms at work here, just show examples.

with ui.div(class="border border-default"):
    ui.h3("This is a title")
    ui.p("This is a paragraph")
    with ui.p():
        "This is a much more complicated paragraph"
        @render.display
        def foo():
            ...

I feel like I'd just look at that and be like, "OK, sure."

What will happen if a user calls a context manager directly or an ordinary function with with?

I have to admit it's news to me that you can't call a context manager directly and just have it tagify. I feel like it'd be trivial to have RecallContextManager support this--if we did, how much would that alleviate your concerns, @gadenbuie?

@gadenbuie
Copy link
Collaborator

I have to admit it's news to me that you can't call a context manager directly and just have it tagify. I feel like it'd be trivial to have RecallContextManager support this--if we did, how much would that alleviate your concerns, @gadenbuie?

I think that would help a lot, as does the fact that that tags will support both forms. As long as nothing bad will happen when users pick the wrong form (except when that causes a truly show-stopping error), then my concerns would be alleviated quite a lot.

@gshotwell
Copy link
Contributor

I think most of the users who are coming from Streamlit or Gradio will have a pretty strong bias for context managers for layout components. So the thing I would worry most about is the case where someone tries to use something as a context manager and it fails because that's the target user's most likely pattern. My view is that we should try to use context managers exclusively in our Express documentation, even if they're not really the only way to lay those apps out. That way the user can have a fairly simple rule of "in Express I should use context managers for layout" even if that's more of a stylistic thing than a coding thing.

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

No branches or pull requests

5 participants