New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a File Uploader Widget #488
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - this looks really good! I left some mostly nit-picky comments on style.
But the big thing missing is tests :)
FileManager
should have its own small test suiteReportSession
should have tests forhandle_new_file
andhandle_file_chunk
(alternately, I think these tests could exist inServer_test.py
- there are a number of tests in there that simulate a Websocket connection from the client, and send test messages to and from that simulated client.)- The widget itself should have a simple Cypress test
Found a bug: the file uploader gets into a "loading forever" state when you pass the wrong kind of file. Steps to repro:
|
@tconkling I fixed all your comments and added a test for @tvst I fixed that bug (I check the file type before start the upload) and added a method to cancel the upload. |
@ajinkya933 that source code is old, you should not use Try to run this:
|
Have you installed streamlit from here: https://github.com/streamlit/streamlit/wiki/Contributing#Ubuntu. I am having trouble installing develop version of streamlit as pointed out by @tconkling initially I run
for which I get a message But when I do run
After getting this error I followed instructions here (https://github.com/streamlit/streamlit/wiki/Contributing#Ubuntu) and heres my output error |
outline: 0; | ||
} | ||
|
||
.stFileUploaderError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only add st
to the top-level classname in each component.
Also, wrap the whole CSS in .stFileUploader {
and then you can just rename this to .uploadError
or something. For example:
.stFileUploader {
...
.uploadError {
...
}
...
}
...same with everything else in this file.
That's what we've been doing in the CSS for other elements.
padding-top: 1rem; | ||
outline: 0px; | ||
-webkit-box-orient: vertical; | ||
-webkit-box-direction: normal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are all these -webkit
properties needed? It's best to avoid browser-specific code.
|
||
// The default file size allowed by server config is 50MB. | ||
// The best message size to don't freeze the server is 10MB | ||
const chunkByteLimit = 10 * 1e6 //10MB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the user sets the limit to 5MB? Does this break anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we have a validation to check if the widget accept the file first
lib/streamlit/DeltaGenerator.py
Outdated
Examples | ||
-------- | ||
>>> file = st.file_uploader("Upload a image", type=["png"]) | ||
>>> if file != None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is not None
lib/streamlit/DeltaGenerator.py
Outdated
|
||
Examples | ||
-------- | ||
>>> file = st.file_uploader("Upload a image", type=["png"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type="png"
lib/streamlit/DeltaGenerator.py
Outdated
---------- | ||
label : str or None | ||
A short label explaining to the user what this file uploader is for. | ||
type : list of string or None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type : str or list of str or None
lib/streamlit/folder_black_list.py
Outdated
@@ -32,6 +33,14 @@ | |||
] | |||
|
|||
|
|||
# Add the Streamlit lib folder when in dev mode, since otherwise we end up with | |||
# weird situations where the ID of a class in one run is the same as in another |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, this should be "is not the same" (I forgot the "not")
i try to run ' pip install -e path/to/code' command but it tells me no setup.py found. |
solved by 'cd streamlit/lib' first |
get same error |
Hey, |
Perhaps you can help me in resolving this issue #728 File uploader feature is available in develop branch but I am having a tough time installing it |
lib/streamlit/caching.py
Outdated
@@ -53,6 +54,18 @@ | |||
to suppress the warning. | |||
""" | |||
|
|||
# based on: https://stackoverflow.com/questions/43506378/how-to-get-source-code-of-function-that-is-wrapped-by-a-decorator | |||
# In Python 2, the @functools.wraps() decorator does not set the convenience __wrapped__ | |||
if sys.version_info[0:2] >= (3, 4): # Python v3.4+? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it might be 3.2
New in version 3.2: Automatic addition of the wrapped attribute.
To allow access to the original function for introspection and other purposes (e.g. bypassing a caching decorator such as lru_cache()), this function automatically adds a wrapped attribute to the wrapper that refers to the original function.
Changed in version 3.4: The wrapped attribute now always refers to the wrapped function, even if that function defined a wrapped attribute. (see issue 17482)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I just read the comment on stackoverflow, so I see why you chose 3.4
lib/streamlit/caching.py
Outdated
if sys.version_info[0:2] >= (3, 4): # Python v3.4+? | ||
wraps = functools.wraps # built-in has __wrapped__ attribute | ||
else: | ||
def wraps(wrapped, assigned=functools.WRAPPER_ASSIGNMENTS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add some tests for python 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, if this doesn't work, a caching test will fail.
Is there a way to install the latest version from this branch of repo directly, since this PR has been merged? Since it has not been included in the latest released version, pip install won't get the most updated version |
@dancebean |
This breaks when using conda and windows. Just saw this issue with Windows and the |
@dancebean, @vanpelt (👋), and @RichardOberdieck : Please note that Also, you can find instructions to build Streamlit from source here. Happy New Year! 🎆 |
Great work! Besides, how to upload an image and write to local as an image file? |
This can do:
|
Great, @universewill . Please also know that |
I get the following error when i run this
|
#120
Example: