Adding utils and tests for hello.py#261
Conversation
lib/streamlit/hello/util.py
Outdated
There was a problem hiding this comment.
It is a variable name in this case.
lib/streamlit/hello/__init__.py
Outdated
There was a problem hiding this comment.
Why is this needed? We could just have cli.py call run(), right?
There was a problem hiding this comment.
Yeah,... well... hello works by grabbing the source file path of hello.py. Not sure it will be ok by just "calling it". Is it just because of re-run? I understand we won't be modifying the source code at runtime in this case, but the API may break.
filename = streamlit.hello.__file__
# For Python 2 when Streamlit is actually installed (make install rather
# than make develop).
if filename.endswith(".pyc"):
filename = "%s.py" % filename[:-4]
_main_run(filename)
There was a problem hiding this comment.
I did ended up removing this and refactoring the imports.
lib/streamlit/hello/util.py
Outdated
There was a problem hiding this comment.
You can get a function's docstring with the_func.__doc__. Wouldn't it be easier / safer to just use that here?
For example:
doc = the_func.__doc__.splitlines()
last_line = len(doc)
return lines[last_line + 1:]
There was a problem hiding this comment.
I can try this. Seems a good option. Not sure on top of my head if the_func.__doc__.splitlines() has to maintain the same formatting and gives the same number of lines, but it may as well be.
There was a problem hiding this comment.
It does not tell me about the lines with """, it only gives me the lines with the actual content of the docstring. So,
"""
"""
and
""" """
are the same. I would still have to parse and do something with these, so not sure of the actual advantage.
There was a problem hiding this comment.
I tried to write it and it quickly became as complex. I would leave it as it is. I am not a big fan of ad hoc logic like this one...
There was a problem hiding this comment.
Honestly there might not be need for doing all this introspection... we could just remove the docstring and factor out the comments, then require all demo functions not to have a docstring (easier assert). The current code happened because of some initial back and forth between me and Adrian, but no reason to stay this way. If you like this approach better I can refactor in this sense, it will take an extra hour or so. ... Evil of introspection...
|
I removed the I also moved the actual demo code out to a separate file |
* develop: Fullscreen button to expand tables and charts (streamlit#137) Blacklist for hashing (streamlit#254) Adding utils and tests for hello.py (streamlit#261) Fix port-finding bugs (streamlit#280) Fix hello.py usage of os.path.join on a URL (streamlit#285)
The purpose of this PR is adding a test for some utility in
hello.py. I created a package forhello.py, modifiedcli.pyand I added a util file to be used byhello.py. I then added a test for such util file.