Skip to content

Fix multiselect docstring example#1442

Merged
tvst merged 3 commits intostreamlit:developfrom
IanCal:develop
May 13, 2020
Merged

Fix multiselect docstring example#1442
tvst merged 3 commits intostreamlit:developfrom
IanCal:develop

Conversation

@IanCal
Copy link
Copy Markdown
Contributor

@IanCal IanCal commented May 12, 2020

Issue: #1441

Description:

The docstring example had three small issues:

  • Added a missing comma at the end of the line
  • Default values (currently) need to be an actual list, so changed from a tuple
  • Order of parameters was wrong, putting the default before the options

Contribution License Agreement

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

The docstring example had three small issues:
* Added a missing comma at the end of the line
* Default values (currently) need to be an actual list, so changed from a tuple
* Order of parameters was wrong, putting the default before the options
@IanCal IanCal requested a review from a team as a code owner May 12, 2020 12:54
@randyzwitch randyzwitch self-requested a review May 12, 2020 13:09
Add in ellipsis for line continuation
@IanCal
Copy link
Copy Markdown
Contributor Author

IanCal commented May 12, 2020

Is the failing cypress tests side an issue? Should I move this over to master rather than develop?

@randyzwitch
Copy link
Copy Markdown
Contributor

No, Cypress isn't an issue.

Copy link
Copy Markdown
Contributor

@tvst tvst left a comment

Choose a reason for hiding this comment

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

Looks good, but please address the comment before merging

('Yellow', 'Red')
... ('Green', 'Yellow', 'Red', 'Blue'))
... ('Green', 'Yellow', 'Red', 'Blue'),
... ['Yellow', 'Red'])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think they should either both be tuples or both be lists.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, I just went and fixed it 😉

@tvst tvst merged commit dbb5c2f into streamlit:develop May 13, 2020
tconkling added a commit that referenced this pull request May 18, 2020
* develop:
  Keras model hash func (#1450)
  Add support for st.echo("below") to print echoed text below the Streamlit output (#1452)
  Update 3.5 references to 3.6 (#1444)
  PyTorch model hash func (#1445)
  Fix link for pipenv in Getting Started (#1455)
  TextArea and TextInput max_chars (#1423)
  Fix multiselect docstring example (#1442)
  Configuring ReadtheDocs (#1435)
  Fix "strip()" bug when internal IP is None (#1434)
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.

3 participants