-
Notifications
You must be signed in to change notification settings - Fork 4k
Clarify set_page_config docstring and exception message #6594
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
Clarify set_page_config docstring and exception message #6594
Conversation
Added clarification per Issue streamlit#6572
willhuang1997
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Love what you do on the streamlit forum and appreciate it so much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @MathCatsAnd!! The change looks good to me.
I was thinking we could also amend the related StreamlitAPIException message in the same PR (and edit the PR title+description). WDYT @willhuang1997?
| "`set_page_config()` can only be called once per app, " |
The above can be changed to
"`set_page_config()` can only be called once per app page, "
Similarly, these two lines in the related e2e test can be updated to add page to the end:
.contains("set_page_config() can only be called once per app") .contains("set_page_config() can only be called once per app")
Updated exceptions and tests to clearly reflect the restriction per page instead of per whole app, consistent with documentation tweak in streamlit#6572
…tsAnd/streamlit into page-config-documentation
|
Thanks @snehankekre. I wasn't sure where all the associated exceptions and tests were. I've added the updates you mentioned to the PR. |
|
Woohoo, thanks for updating the exception and tests, @MathCatsAnd ! 😄 |
|
I've updated the top comment as well @snehankekre. Feel free to update the name of the commit as desired. I'm not 100% confident in my naming sense, yet. If I were to guess, maybe: |
* develop: (22 commits) enzyme -> react-testing-library (Countdown, Modal, ProgressBar) (streamlit#6744) Remove console.log (streamlit#6753) Update `st.data_editor` and `st.dataframe` docstrings (streamlit#6752) Update `st.data_editor` session state format (streamlit#6711) Cypress flaky test fixes (streamlit#6743) Document integer size limit for number_input and slider (streamlit#6724) Change default theme hash on app init (streamlit#6729) Use .genericFonts instead of .fonts to fix bug with theme postMessage. (streamlit#6732) Improve startup performance by lazy loading some dependencies (streamlit#6531) Add support for Altair 5 (streamlit#6618) Update modals (streamlit#6688) Improve docstrings for `ttl` and `max_entries` (streamlit#6733) Improve `st.columns` docstring (streamlit#6727) Removed orphan line in dataframe docstring (streamlit#6734) Fix useIsOverflowing dependency array (streamlit#6731) Remove experimental from data editor (streamlit#6712) Clarify set_page_config docstring and exception message (streamlit#6594) Add a config option to disable warning for setting both a widget default and its key in session_state (streamlit#6640) Add column configuration API for `st.dataframe` and `st.data_editor` (streamlit#6598) Migrate datetime column formatting from date-fns to momentJS (streamlit#6702) ...
📚 Context
Added documentation and exception clarification per Issue #6572
🧠 Description of Changes
Updated a few words in the
set_page_configdocstring.Fixed associated exception message and tests to reflect being restricted to one call per page instead of per (whole) app.
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.