Skip to content

Conversation

@MathCatsAnd
Copy link
Contributor

@MathCatsAnd MathCatsAnd commented Apr 28, 2023

📚 Context

Added documentation and exception clarification per Issue #6572

🧠 Description of Changes

Updated a few words in the set_page_config docstring.
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.

@snehankekre snehankekre self-requested a review April 28, 2023 12:46
Copy link
Contributor

@willhuang1997 willhuang1997 left a 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!

@willhuang1997 willhuang1997 added the security-assessment-completed Security assessment has been completed for PR label Apr 28, 2023
Copy link
Contributor

@snehankekre snehankekre left a 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:

Updated exceptions and tests to clearly reflect the restriction per page instead of per whole app, consistent with documentation tweak in streamlit#6572
@MathCatsAnd
Copy link
Contributor Author

MathCatsAnd commented May 1, 2023

Thanks @snehankekre. I wasn't sure where all the associated exceptions and tests were. I've added the updates you mentioned to the PR.

@snehankekre
Copy link
Contributor

Woohoo, thanks for updating the exception and tests, @MathCatsAnd ! 😄

@MathCatsAnd
Copy link
Contributor Author

MathCatsAnd commented May 2, 2023

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:
Fix set_page_config documentation & exception

@snehankekre snehankekre changed the title Clarify set_page_config documentation Clarify set_page_config docstring and exception message May 10, 2023
@snehankekre snehankekre added the type:docs Requests for changes to docs (will be rerouted to docs repo) label May 10, 2023
@snehankekre snehankekre merged commit d2063cf into streamlit:develop May 22, 2023
@MathCatsAnd MathCatsAnd deleted the page-config-documentation branch May 22, 2023 06:38
tconkling added a commit to tconkling/streamlit that referenced this pull request May 30, 2023
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-assessment-completed Security assessment has been completed for PR type:docs Requests for changes to docs (will be rerouted to docs repo)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants