Support cli parameters by dnuske · Pull Request #186 · streamlit/streamlit

dnuske · 2019-09-25T06:25:19Z

issue: https://github.com/streamlit/streamlit-old-private/issues/670

Brief

Fetches all config options available and exposes them as click cmd option params.

Description

You can do things such as streamlit run --server.port 4031 examples/reference.py now
To see all available options call streamlit run --help.

👆I don't consider it looks great, I'd prefer it to have a break line after every option, but wasn't able to find a way to format the description for an option in click.

Things that keep this away from being perfect:

we can add a nice error when type casting fails (like when sending --server.port "asd")
better format to streamlit run --help

Weird behavior

something that seems unusual to me is that calling config._set_option only works when the option is not existing in the config file! 🤔

tvst

Approved, once comments are addressed.

So excited for this to land 💯

tvst · 2019-10-03T06:47:32Z

lib/streamlit/cli.py

+def _compose_option_parameter(config_option):
+    """
+    Composes given config option options as options for click lib.
+    """


Fix docstring to match the Numpy docstring style guide.

tvst · 2019-10-03T06:48:51Z

lib/streamlit/cli.py

+    """
+    Decorator that adds config param keys to click dynamically.
+    """
+    for _, value in reversed(_config._config_options.items()):


Why do you need reversed here?

for some reason the default order gives me the last config item first. I don't know why I just saw it was this way and reversed it.

Oh actually it makes sense now. click probably reverses them internally so the help string appears in the same order as you declare the decorators. The topmost decorator should show up first on the help.

But the topmost decorator is actually the last decorator that is applied. So on our case, we should reverse again to make sure the thing we want to have at the top is applied last.

tvst · 2019-10-03T06:50:21Z

lib/streamlit/cli.py

    _main_run(filename)


+def _compose_option_parameter(config_option):


I think a name like this would be clearer: _convert_config_option_to_click_option

tvst · 2019-10-03T06:51:10Z

lib/streamlit/cli.py

    from validators import url

+    # read through all option configs that are set via cmd option param
+    # and call set_option to override default/file configs


Nit: capitalize comment.

tvst · 2019-10-03T06:52:44Z

lib/streamlit/cli.py

    """Run a Python script, piping stderr to Streamlit.
    The script can be local or it can be an url. In the
    latter case, streamlit will download the script to a
    temporary file and runs this file.


This is not your doing, but can you make this docstring extend all the way to the 80-character line?

tvst · 2019-10-03T06:55:01Z

lib/streamlit/cli.py

    from validators import url

+    # read through all option configs that are set via cmd option param
+    # and call set_option to override default/file configs


I don't really understand this comment 😕

Can you rephrase? Something like: "The "streamlit run" command supports passing Streamlit's config options as flags. So here we read through all unhandled command-line parameters, massage them, and pass them to _set_config()

"massage them" 😂

tvst · 2019-10-03T06:56:17Z

lib/streamlit/cli.py


+    # read through all option configs that are set via cmd option param
+    # and call set_option to override default/file configs
+    for config_option in kwargs:


Can you move this whole for-loop into its own helper function? Something like _apply_config_options_from_cli()

tvst · 2019-10-03T06:56:50Z

lib/streamlit/cli.py

+
+
 @main.command("run")
+@configurator_options


We should add this to streamlit hello too.

tvst · 2019-10-04T23:10:39Z

lib/streamlit/DeltaGenerator.py

            if old_step is None or old_stop is None:
-                raise AttributeError("'RangeIndex' object has no attribute "
-                                     "'step'")
+                raise AttributeError("'RangeIndex' object has no attribute " "'step'")


The linter messed up here. If it fits within 80 characters, can you join these strings?

doesn't fit :/

raise AttributeError("'RangeIndex' object has no attribute " "'step'")

* develop: Support cli parameters (streamlit#186) Update README.md Update README.md (streamlit#282) Bug fix: make Streamlit watch files in subfolders again (streamlit#265) fix st.code("") (streamlit#272) different jslint commands for circleci and not (streamlit#273) Batch updates to report elements in the App state (streamlit#194) Minor typo in pull_request_template.md (streamlit#262) Fix wrong quote used in the provided sample code (streamlit#267) Fix websocket port handling (streamlit#263) Closing sidebar when clicking outside (streamlit#223) Version 0.47.2 (streamlit#212) [docs] Changed value in Show progress section of docs/getting_started.md to fix error (Issue streamlit#234) (streamlit#235) Update README.md Remove checkbox from CLA in PR Template Update README.md Update README.md mapping_demo: catch urllib errors (streamlit#224)

Support cli parameters (#186)

* develop: Read config from ${CWD}/.streamlit/config.toml (streamlit#227) Fix unsafe_allow_html (streamlit#259) Support cli parameters (streamlit#186) Update README.md Update README.md (streamlit#282) Bug fix: make Streamlit watch files in subfolders again (streamlit#265) fix st.code("") (streamlit#272) different jslint commands for circleci and not (streamlit#273) Batch updates to report elements in the App state (streamlit#194) Minor typo in pull_request_template.md (streamlit#262) Fix wrong quote used in the provided sample code (streamlit#267) Fix websocket port handling (streamlit#263) Closing sidebar when clicking outside (streamlit#223) Version 0.47.2 (streamlit#212) [docs] Changed value in Show progress section of docs/getting_started.md to fix error (Issue streamlit#234) (streamlit#235) Update README.md

dnuske added 2 commits September 25, 2019 02:53

add all existing config options as click cmd param options

40af6cf

add description and deprecation notice to config param options

220c25f

dnuske requested review from kantuni, monchier and tvst September 25, 2019 06:25

dnuske self-assigned this Sep 25, 2019

dnuske requested review from arraydude, dcaminos, jrhone, tconkling and treuille September 25, 2019 06:25

dnuske removed their assignment Sep 25, 2019

dnuske added 7 commits September 25, 2019 18:07

adds type to ConfigOptions to properly type cast the cmd option params

1679c4b

lint

0e9a127

Merge branch 'develop' into support-cli-parameters

8d0f93a

lint

1a9a8f1

display config option keys --help in the same order they were created

32a581a

adds test to compose_option_parameter for cli cmd config options

c2a528e

lint

84b16ae

tvst approved these changes Oct 3, 2019

View reviewed changes

dnuske added 5 commits October 4, 2019 09:36

Merge branch 'develop' into support-cli-parameters

d80730a

lint

b4710cf

Merge branch 'develop' into support-cli-parameters

4a34bf6

moved logic to parse config options out of main_run

2c2adc9

lint

4f3ad67

dnuske requested a review from tvst October 4, 2019 15:15

tvst approved these changes Oct 4, 2019

View reviewed changes

fix tests for cli config option params on py2.7

306d3a0

dnuske merged commit 2bbb67a into streamlit:develop Oct 7, 2019

dnuske deleted the support-cli-parameters branch October 7, 2019 19:31

sthagen referenced this pull request in sthagen/streamlit-streamlit Oct 8, 2019

Merge pull request #2 from streamlit/develop

28c749d

Support cli parameters (#186)

dnuske mentioned this pull request Oct 11, 2019

add cli config flags to streamlit hello #376

Closed

		_main_run(filename)


		def _compose_option_parameter(config_option):



		@main.command("run")
		@configurator_options

Conversation

dnuske commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Brief

Description

Things that keep this away from being perfect:

Weird behavior

Uh oh!

tvst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnuske Oct 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dnuske commented Sep 25, 2019 •

edited

Loading

dnuske Oct 7, 2019 •

edited

Loading