Unify width and height related docstrings #8441

LukasMasuch · 2024-04-05T15:45:22Z

Describe your changes

This PR unifies the docstrings of the width, height and use_container_width parameters across our commands.

Contribution License Agreement

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

lib/streamlit/elements/arrow_altair.py

lib/streamlit/elements/arrow_vega_lite.py

sfc-gh-dmatthews · 2024-04-20T16:43:53Z

lib/streamlit/elements/arrow.py

+            Desired width of the dataframe expressed in pixels. If None (default),
+            selects the width automatically.


Suggested change

Desired width of the dataframe expressed in pixels. If None (default),

selects the width automatically.

Desired width of the dataframe expressed in pixels. If ``width`` is

``None`` (default), Streamlit selects the width automatically.

Or possibly:

Suggested change

Desired width of the dataframe expressed in pixels. If None (default),

selects the width automatically.

Desired width of the dataframe expressed in pixels. If ``width`` is

``None`` (default), Streamlit sets the dataframe width to fit its

contents up to the width of the parent container. If ``width`` is

greater than the width of the parent container, Streamlit sets the

dataframe width to match the width of the parent container.

One thing I've been grappling with recently is standardizing the grammar within docstrings. We have a mix of fragments and complete sentences and I want to consciously draw a line for when we use each. I've been kicking around the idea that we let the first "sentence" be a noun phrase, stated as a definition, but then switch to complete sentences thereafter if we're going to describe behavior or cases.

I'm a little hesitant that that might be too extreme and get too wordy when applied broadly, so I'm curious about other opinions here. (As an aside, I also want to make sure Python objects are formatted as code. This isn't consistent everywhere yet, but I'm inclined to use None in code format.)

When I poke around other libraries, I don't see a lot of consistency elsewhere. Our parameter descriptions tend to be comparatively more complex with a lot more switching behavior, too.

I also need to have a separate discussion about "dataframe" vs "DataFrame" since pandas, Arrow, and Spark all use the capitalized form. It'd be nice to have an uncapitalized, generic version (and we mostly do in the docs), but I've had a hard time finding anyone else doing that in any official documentation. Maybe we can just choose to lead the charge. 😅

Regarding: I think there was some discussion some time ago related to dataframe vs DataFrame. I believe the outcome was that we only use DataFrame if we are referring to a specific implementation of a dataframe, e.g. Pandas DataFrame, Snowpark DataFrame. But if we are talking about this topic more high-level or if we are talking about the actual table UI we are using lowercased dataframe version. But not sure if we ever made a decision on that.

The suggested docstring sounds good to me 👍 and it would be awesome to have this kind of general guidelines on how we write / structure docstrings

sfc-gh-dmatthews · 2024-04-20T20:13:45Z

lib/streamlit/elements/arrow.py

-            Desired height of the dataframe expressed in pixels. If None, a
-            default height is used.
+            Desired height of the dataframe expressed in pixels. If None (default),
+            selects the height automatically.

        use_container_width : bool
            If True, set the dataframe width to the width of the parent container.


Suggested change

If True, set the dataframe width to the width of the parent container.

Whether to override ``width`` with the width of the parent container.

The default is ``False``. If ``use_contatiner_width`` is ``True``,

Streamlit sets the width of the dataframe to match the width of the

parent container.

or possibly:

Suggested change

If True, set the dataframe width to the width of the parent container.

Whether to override ``width`` with the width of the parent container.

The default is ``False``. If ``use_contatiner_width`` is ``True``,

Streamlit ignores ``width`` and sets the width of the dataframe to

match the width of the parent container.

To follow through with my above comment, this argument could be something like this. (It didn't explicitly state its default before.) Note that the comment isn't letting me include next line. I'm proposing the "overrides" language in place of "takes precedence." GitHub wouldn't let me include line 94 in the comment. :)

github-actions · 2024-05-13T01:50:32Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

sfc-gh-dmatthews · 2024-05-13T15:18:26Z

I'll get the edits added per our conversation.

LukasMasuch · 2024-05-13T15:42:28Z

@sfc-gh-dmatthews Sounds good; sorry that I didn't get to this ... I'm fully focused on wrapping up selections for the 1.35 release.

sfc-gh-dmatthews · 2024-05-13T16:10:38Z

No worries. I had intended to update the PR and just got busy on the rest of 1.34.0 and hadn't circled back yet.

sfc-gh-dmatthews · 2024-05-22T18:47:26Z

lib/streamlit/elements/arrow_altair.py

+        width=width or 0,
+        height=height or 0,


@LukasMasuch I'm copying these edits into my PR, but I'm not sure what this line is and I'm a little less confident copying into where I think it goes. I found the built_in_chart_utils.py file and I updated the parts in def _generate_chart to have int | None = None. What is this part exactly that previously assigned an int and is not assigning a bool?

You could either use the width or 0 whenever generate_chart is called in vega_charts.py, or allow None in generate_chart for width and height and add this into the top of generate_chart function:

width = width or 0 height = height or 0

this basically says that its guaranteed to be a number or 0

## Describe your changes Docstring updates for release 1.35.0. * New selection feature added for `st.dataframe`, `st.plotly_chart`, `st.altair_chart`, and `st.vega_lite_chart`. * New event and selection states (dictionary schemas) * New function: `st.logo` * Additional docstring edits for other chart functions for consistency. This PR copies docstring edits from #8441. In particular, there is a small change in parameter types for width and height (previously `int` with an initial value of 0, and now `int | None = None`. This is the one thing outside of docstrings. ## GitHub Issue Link (if applicable) ## Testing Plan None. --- **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-dmatthews · 2024-05-23T16:27:47Z

Closing this PR since the edits were copied into the docstrings for 1.35.0.

Unify width related docstrings

a095ba4

LukasMasuch added security-assessment-completed impact:users change:docs labels Apr 5, 2024

LukasMasuch requested a review from sfc-gh-dmatthews April 5, 2024 15:46

LukasMasuch added 2 commits April 5, 2024 17:52

Add correct code blocks

3fa5334

Use correct code block

f0a0c8c

willhuang1997 approved these changes Apr 5, 2024

View reviewed changes

willhuang1997 reviewed Apr 5, 2024

View reviewed changes

lib/streamlit/elements/arrow_altair.py Outdated Show resolved Hide resolved

willhuang1997 reviewed Apr 5, 2024

View reviewed changes

lib/streamlit/elements/arrow_vega_lite.py Outdated Show resolved Hide resolved

LukasMasuch added 2 commits April 18, 2024 17:19

Apply pr feedback

1fb7841

Merge branch 'develop' into docs/unify-width-height-docstring

a311b56

sfc-gh-dmatthews reviewed Apr 20, 2024

View reviewed changes

LukasMasuch mentioned this pull request May 3, 2024

Refactor vega-related charting #8595

Merged

github-actions bot added the stale label May 13, 2024

github-actions bot removed the stale label May 14, 2024

sfc-gh-dmatthews reviewed May 22, 2024

View reviewed changes

sfc-gh-dmatthews mentioned this pull request May 22, 2024

Docstrings for 1.35.0 (and type consistency for charts) #8740

Merged

sfc-gh-dmatthews closed this May 23, 2024

sfc-gh-dmatthews deleted the docs/unify-width-height-docstring branch May 23, 2024 16:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify width and height related docstrings #8441

Unify width and height related docstrings #8441

LukasMasuch commented Apr 5, 2024 •

edited

sfc-gh-dmatthews Apr 20, 2024 •

edited

sfc-gh-dmatthews Apr 20, 2024

LukasMasuch Apr 28, 2024 •

edited

LukasMasuch Apr 28, 2024

sfc-gh-dmatthews Apr 20, 2024 •

edited

github-actions bot commented May 13, 2024

sfc-gh-dmatthews commented May 13, 2024 •

edited

LukasMasuch commented May 13, 2024

sfc-gh-dmatthews commented May 13, 2024

sfc-gh-dmatthews May 22, 2024 •

edited

LukasMasuch May 22, 2024

sfc-gh-dmatthews commented May 23, 2024

		Desired width of the dataframe expressed in pixels. If None (default),
		selects the width automatically.

-            Desired width of the dataframe expressed in pixels. If None (default),
-            selects the width automatically.
+            Desired width of the dataframe expressed in pixels. If ``width`` is
+            ``None`` (default), Streamlit sets the dataframe width to fit its
+            contents up to the width of the parent container. If ``width`` is
+            greater than the width of the parent container, Streamlit sets the
+            dataframe width to match the width of the parent container.

-            If True, set the dataframe width to the width of the parent container.
+            Whether to override ``width`` with the width of the parent container.
+            The default is ``False``. If ``use_contatiner_width`` is ``True``,
+            Streamlit sets the width of the dataframe to match the width of the
+            parent container.

Unify width and height related docstrings #8441

Unify width and height related docstrings #8441

Conversation

LukasMasuch commented Apr 5, 2024 • edited

Describe your changes

sfc-gh-dmatthews Apr 20, 2024 • edited

Choose a reason for hiding this comment

sfc-gh-dmatthews Apr 20, 2024

Choose a reason for hiding this comment

LukasMasuch Apr 28, 2024 • edited

Choose a reason for hiding this comment

LukasMasuch Apr 28, 2024

Choose a reason for hiding this comment

sfc-gh-dmatthews Apr 20, 2024 • edited

Choose a reason for hiding this comment

github-actions bot commented May 13, 2024

sfc-gh-dmatthews commented May 13, 2024 • edited

LukasMasuch commented May 13, 2024

sfc-gh-dmatthews commented May 13, 2024

sfc-gh-dmatthews May 22, 2024 • edited

Choose a reason for hiding this comment

LukasMasuch May 22, 2024

Choose a reason for hiding this comment

sfc-gh-dmatthews commented May 23, 2024

LukasMasuch commented Apr 5, 2024 •

edited

sfc-gh-dmatthews Apr 20, 2024 •

edited

LukasMasuch Apr 28, 2024 •

edited

sfc-gh-dmatthews Apr 20, 2024 •

edited

sfc-gh-dmatthews commented May 13, 2024 •

edited

sfc-gh-dmatthews May 22, 2024 •

edited