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

feat: Support templates with _template.json metadata #1631

Merged
merged 24 commits into from
Aug 27, 2024

Conversation

gadenbuie
Copy link
Collaborator

@gadenbuie gadenbuie commented Aug 22, 2024

This PR refactors shiny create so that it now recognizes a _template.json file. In short, every template that we create should now have an associated _template.json.

The _template.json for our internal templates looks like this:

{
  "id": "app",
  "name": "basic-sidebar",
  "title": "Sidebar layout",
  "description": "An app with inputs in a sidebar and a plot in the main area."
}

Edit: the "id" field was originally "name" at the start of this PR, but I landed on "id" because the value should be unique within a repo of templates. Also "name" and "title" are ambiguous but "id" and "title" are more clearly distinct.

In general, only id is required, but id, title, and description are expected to be the most commonly included fields. Additional fields can be added but will be ignored by the internal ShinyTemplate data type.

We differentiate between "type": "app" templates and "type": "package" templates, although I don't think there are external package-templates at this time. In the future, type could be used for other templates, e.g. "type": "module" would be interesting.

I've also updated the py-shiny-templates repo to use the _template.json file (see posit-dev/py-shiny-templates#23), so that shiny create can now list templates in an external github repo:

❯ shiny create --github posit-dev/py-shiny-templates@template-json
ℹ Using GitHub repository posit-dev/py-shiny-templates@template-json.
? Which template would you like to use?: (Use arrow keys)
 » A Basic app
   Basic reactive plot
   Navigating multiple panels
   Reactive plot in sidebar
   Basic dashboard
   Restaurant tips dashboard
   SQL database explorer
   Location distance calculator
   Model scoring
   Streaming database updates
   Streaming file updates
   Streaming folder updates
   NBA player career comparisons
   Article on regularization in ML
   Stock price tracker
   Survey form
   Survey wizard form
   [Cancel]

Note that the menu ordering is driven by sorted path names, so I renamed our internal templates to use 01-...., 02-...., etc. The template "name" in the JSON file is expected to be unique, but we don't do any validation here and instead return the first template with a matching name. We do print out confirmation of which template we're using so there is an opportunity to notice a problem in advance.

Generative AI Templates

While here, I also added the generative AI examples to the default shiny create interface:

shiny-create-chat-ai

Used both internally and for external template repos provided via `--github` command line option
* Rename `use_internal_template()` (replaces `use_template_internal()`)
* Separate `js_component_questions()` into
    * `use_internal_package_template()`: questions related to picking an internal package template
    * `package_template_questions()`: questions related to setting up and copying the package template
@gadenbuie gadenbuie requested a review from cpsievert August 22, 2024 19:05
type: str = "text",
):
self.text = text
self.type: Literal["action", "info", "warning", "danger", "text"] = "text"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not have the Literal type in the function signature?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the incoming value is from untyped JSON

Comment on lines +312 to +327
def use_internal_chat_ai_template(
input: str | None = None,
dest_dir: Optional[Path] = None,
package_name: Optional[str] = None,
):
if input is None:
input = questionary.select(
"Which kind of generative AI template would you like to use?",
choices=[
Choice(title="By provider...", value="_chat-ai_hello-providers"),
Choice(title="Enterprise providers...", value="_chat-ai_enterprise"),
back_choice,
cancel_choice,
],
style=styles_for_questions,
).ask()
Copy link
Collaborator

@cpsievert cpsievert Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't feel great to be "exporting" _templates.json with no way to support choice groups (i.e., sub-menus).

If there was one _templates.json per shiny create, it'd be fairly easy to support choice groups, but with one _templates.json per app, it's definitely harder.

Would you mind reminding me of the benefits of one _templates.json per app?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so much easier to work with a single json file per template than in one large JSON file. With atomic template metadata, it's much easier to keep track of the metadata for the template. For example, I just moved the chat templates from examples/chat to shiny/templates/chat and had to change one string in our code base. Then I renamed shiny/templates/app-templates to shiny/template/app and again made one small change.

With the --github flag, it's easy to do one-level choice groups with the current setup. Imagine we put the chat templates in py-shiny-templates. Then you can construct a shiny create command that only lists those chat templates, ignoring everything else in the repo:

shiny create --github posit-dev/py-shiny-templates:chat/hello-providers

We could certainly implement choice groups using an additional metadata file, but organizing templates by directory with atomic template files is pretty powerful. In practice, I think we'd use the GitHub repo path formulation to limit choices for a certain type of template within a template repo.

Overall, this approach feels like a good 80% of what we want without rewriting from scratch, over investing in abstractions, or closing off the opportunity to build on this in the future.

I guess the big thing is that, if we find we have a need we can easily extend the current approach to build submenus. I certainly see the value in having support for submenus, but I don't think the need is strong enough at this moment for it to be needed.

Copy link
Collaborator

@cpsievert cpsievert Aug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

easily extend the current approach to build submenus

If you have something in mind, would you mind writing it up in an issue? It would be really nice if shiny create --github posit-dev/py-shiny-templates gave you a similar submenu interface (especially if we keep adding templates, it's going to feel overwhelming).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -4,7 +4,7 @@
"title": "Intermediate dashboard",
"description": "An intermediate dashboard with value boxes, several plots in cards and a sidebar.",
"next_steps": [
"Run the app with `shiny run app.py`."
"Run the app with `shiny run app.py` from the app directory."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, the suggestion I made for this was intended for all the templates that have this next step (i.e., let's do them all or none)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Honestly, I don't love this. I think next_steps is needed generally, but this advice is specific to our internal templates where we can write more direct instructions using questionary.

e.g. I'd rather we offer to open the app.py in VS Code or Positron, or not in an editor, at which point our advice for running the app would change.

At the very least we could actually include the app directory in the instructions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, only the basic-app template needs this instruction, otherwise we already have instructions to move into the app directory. (88d4c9f)

❯ shiny create            
? Which template would you like to use? Intermediate dashboard
… Creating Intermediate dashboard Shiny app...
? Enter destination directory: ./04-dashboard-tips
? Would you like to use Shiny Express? Yes
✓ Created Shiny app at 04-dashboard-tips

→ Next steps:
- Install required dependencies:
    cd 04-dashboard-tips
    pip install -r requirements.txt
- Open and edit the app file: 04-dashboard-tips/app.py
- Run the app with `shiny run app.py`.

ℹ Just getting started with Shiny?
→ Learn more at https://shiny.posit.co/py/docs/user-interfaces.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI we keep the full path in

- Open and edit the app file: 04-dashboard-tips/app.py

because VS Code makes this path clickable. If we changed it to app.py we'd break click-to-open. But I think it's reasonable to expect the working directory change to carry over to the shiny run command.

@gadenbuie
Copy link
Collaborator Author

Just remembered that I want to change the name field to id. It needs to be unique, is used as the identifier in the TUI and name is ambiguous with title.

{
  "type": "app",
  "id": "basic-sidebar",
  "title": "Sidebar layout",
  "description": "An app with inputs in a sidebar and a plot in the main area."
}

@gadenbuie gadenbuie force-pushed the feat/create-template-json branch from 5b4d619 to bda4010 Compare August 27, 2024 18:22
@gadenbuie gadenbuie merged commit 0f7765b into main Aug 27, 2024
48 checks passed
@gadenbuie gadenbuie deleted the feat/create-template-json branch August 27, 2024 21:25
schloerke added a commit to machow/py-shiny that referenced this pull request Sep 5, 2024
* main:
  CI(deploy): Add more installation configs to surface failure cause (posit-dev#1658)
  `Chat.messages()` no longer trims messages by default (posit-dev#1657)
  tests(controllers): Split _controls.py into separate files (posit-dev#1652)
  Chat tweaks (posit-dev#1607)
  docs: Add modal_show/remove examples (posit-dev#1628)
  Delay sending of chat UI messages until reactive graph is flushed (posit-dev#1593)
  ci(remove future behavior warning): Set the `asyncio_default_fixture_loop_scope` to `fixture` (posit-dev#1655)
  bug: Verify mypy can run on CI (posit-dev#1650)
  Fix CI install failures on Windows (posit-dev#1651)
  Quartodoc 0.7.6 (posit-dev#1636)
  Allow `@chat.transform_assistant_response` function to return `None` (posit-dev#1641)
  feat: Support templates with `_template.json` metadata (posit-dev#1631)
  Fix KeyError when serving static files (posit-dev#1648)
  tests(navsets): Add navsets kitchensink tests (posit-dev#1602)
  Change default claude model to 3.5 sonnet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants