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

More API changes #69

Merged
merged 24 commits into from
Feb 10, 2022
Merged

More API changes #69

merged 24 commits into from
Feb 10, 2022

Conversation

cpsievert
Copy link
Collaborator

@cpsievert cpsievert commented Feb 7, 2022

Follow up of sorts to #51

  • Define __all__ in shiny/__init__py to better define what from shiny import * does:
    1. The main benefit is that we won't clutter the global namespace with stuff that most apps won't need (e.g., submodules like reactcore, etc)
      2. In general, I think we should avoid listing submodules in __all__ and instead list either sub-packages (i.e., ui) or the the most important objects within submodules (e.g., App, render_plot, etc).
      a. The "down-side" is that you have to from shiny import foo to access a submodule, but by being explicit about it, it feels like a good thing considering (3):
      3. It seems very difficult for users to know what the "true" public-api for a submodule. It seems, by default, most dev environments will list all objects that don't have a _ prefix in that sub-module. For example, this is you get for a tab-completion experience on main right now when typing @reactive.[TAB]

Screen Shot 2022-02-09 at 11 54 20 AM

  • Define __all__ in every submodule (closes Need to define __all__ for each .py file #56)

    • Note this only impacts the result of from shiny.foo import *. It doesn't necessary discourage from shiny.foo import bar (unless bar is imported from another submodule -- at least by default with pylance)
    • For the reason above, we should also flag more objects as private with a _ prefix (there doesn't seem to be a way to say "this thing is internal to shiny, but fine to use across shiny's submodules", but I'd rather err on the side of discouraging users to use it)
  • Moves all UI related functions (even "dynamic UI" things like notifications, progress, insert/remove) to the ui sub-package (see More API changes #69 (comment))

@cpsievert cpsievert changed the title Tidy up the ui submodule namespace Tidy up namespacing Feb 7, 2022
@cpsievert cpsievert requested a review from wch February 7, 2022 23:17
shiny/__init__.py Outdated Show resolved Hide resolved
shiny/types.py Outdated Show resolved Hide resolved
shiny/ui/__init__.py Outdated Show resolved Hide resolved
shiny/ui/insert.py Outdated Show resolved Hide resolved
shiny/ui/input_select.py Outdated Show resolved Hide resolved
…dule in the name; add MISSING_TYPE to public API; add more htmltools to shiny.ui package
…to pull all session dependent ui into another sub-package later on)
@cpsievert cpsievert marked this pull request as draft February 9, 2022 15:58
@cpsievert cpsievert changed the title Tidy up namespacing More API changes Feb 9, 2022
shiny/_fileupload.py Outdated Show resolved Hide resolved
shiny/types.py Outdated Show resolved Hide resolved
@cpsievert cpsievert marked this pull request as ready for review February 10, 2022 20:00
@wch wch merged commit 996798f into main Feb 10, 2022
@wch wch deleted the tidy-namespace branch February 10, 2022 20:00
cpsievert added a commit that referenced this pull request Feb 10, 2022
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.

Need to define __all__ for each .py file
2 participants