Skip to content

Conversation

@sfc-gh-jrieke
Copy link
Contributor

Makes the docstring for ttl on st.cache_data and st.cache_resource a bit nicer to read by explicitly listing all options. I moved the new str option before timedelta because I think it will be used more (and we should promote it!). Also changed a few nits in the max_entries docstring while I was at it.

📚 Context

  • What kind of change does this PR introduce?

    • Documentation

🧠 Description of Changes


Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-jrieke sfc-gh-jrieke requested a review from snehankekre May 23, 2023 01:37
@snehankekre snehankekre added the security-assessment-completed Security assessment has been completed for PR label May 23, 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.

@sfc-gh-jrieke We use reStructuredText in our docstrings. Code should be enclosed in two backquotes, not single. Single backquotes are for italics: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#inline-markup

Additionally, you've switched to Markdown for inline external links. They should again be in reST format: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#external-links

Pending the above changes, the bulleted list looks SO much better. Thanks, Johannes!

@snehankekre snehankekre self-requested a review May 23, 2023 12:15
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 @sfc-gh-jrieke!

Revised

image

Current

image

Copy link
Collaborator

@lukasmasuch lukasmasuch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lukasmasuch lukasmasuch merged commit 4b41973 into develop May 23, 2023
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)
  ...
@vdonato vdonato deleted the improve-ttl-docstrings branch November 1, 2023 23:58
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants