Skip to content

chore(uvicorn): Remove uv source of uvicorn PR #1950

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

Merged
merged 4 commits into from
Apr 1, 2025
Merged

Conversation

schloerke
Copy link
Collaborator

@schloerke schloerke commented Apr 1, 2025

With encode/uvicorn#2602, the expected behavior is achieved and this PR's changes in _main.py may be reverted.


We are close to a release and can not have a uv source in the pyproject.toml. Therefore, we're removing it and extending the reload-excludes to exclude any file within shiny_bookmarks up to 5 sub-folders deep. This should be a near 100% coverage.

@schloerke schloerke self-assigned this Apr 1, 2025
@schloerke
Copy link
Collaborator Author

schloerke commented Apr 1, 2025

Reprex express app

import pandas as pd

from shiny import reactive
from shiny.bookmark import BookmarkState
from shiny.express import app_opts, input, module, render, session, ui
from shiny.types import FileInfo

app_opts(bookmark_store="server")


@session.bookmark.on_bookmarked
async def _(url: str):
    await session.bookmark.update_query_string(url)


@module
def mymod(input, output, session):
    @reactive.effect
    @reactive.event(input.file1, ignore_init=True)
    async def _():
        # When the user interacts with the input, we will bookmark the state.
        await session.bookmark()

    @session.bookmark.on_bookmark
    async def _(state: BookmarkState):
        # After saving state, we will update the query string with the bookmark URL.
        print("Writing to file")
        (state.dir / "example.py").write_text("here!")

    ui.input_file("file1", "Choose File", multiple=True)

    @render.code
    def parsed_file():
        return str(input.file1())


mymod("test")

When uploading a file, it saves the bookmark state. Before the PR will cause a reload. With PR, layers are covered up to 5 module layers deep.

@schloerke schloerke requested a review from Copilot April 1, 2025 15:28
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the custom uvicorn source configuration from pyproject.toml and reverts corresponding code adjustments in _main.py, while also cleaning up redundant nonlocal/global declarations in tests and API examples. The changes aim to rely on the upstream fixes from uvicorn PR #2602 and extend reload exclusion rules for the shiny bookmarks folder.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/pytest/test_reactives.py Removed unnecessary nonlocal declarations to simplify access to outer-scope lists.
shiny/bookmark/_bookmark.py Updated comments to clarify the purpose of the folder and remove outdated TODO notes.
shiny/api-examples/notification_show/app-express.py Removed redundant global declarations in notification callbacks.
shiny/api-examples/notification_show/app-core.py Removed redundant nonlocal declarations in notification callbacks.
shiny/_main.py Extended reload_excludes to ignore files within the shiny_bookmarks folder up to 5 sub-folders deep.
pyproject.toml Removed the uvicorn source configuration as it is no longer necessary.

@schloerke schloerke marked this pull request as ready for review April 1, 2025 15:29
@schloerke schloerke requested a review from cpsievert April 1, 2025 15:29
@schloerke schloerke enabled auto-merge (squash) April 1, 2025 15:30
@schloerke schloerke merged commit dc0761b into main Apr 1, 2025
61 checks passed
@schloerke schloerke deleted the bookmark-patch branch April 1, 2025 15:41
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.

1 participant