From 7df6dfe931c67a0287222f0a7521c683cf42ad6b Mon Sep 17 00:00:00 2001 From: kajarenc Date: Wed, 12 Jan 2022 01:54:19 +0400 Subject: [PATCH 01/10] Add __eq__ method to UploadedFile, to be able to compare different UploadedFile instances with same FileRec. We need that change to prevent `on_change` call for file_uploader on rerun. --- lib/streamlit/uploaded_file_manager.py | 5 +++++ lib/tests/streamlit/file_uploader_test.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/streamlit/uploaded_file_manager.py b/lib/streamlit/uploaded_file_manager.py index 08ed2fdd4ab6..386541354ff4 100644 --- a/lib/streamlit/uploaded_file_manager.py +++ b/lib/streamlit/uploaded_file_manager.py @@ -50,6 +50,11 @@ def __init__(self, record: UploadedFileRec): self.type = record.type self.size = len(record.data) + def __eq__(self, other): + if other is not None: + return self.id == other.id + return False + def __repr__(self) -> str: return util.repr_(self) diff --git a/lib/tests/streamlit/file_uploader_test.py b/lib/tests/streamlit/file_uploader_test.py index 975750073eec..703848693897 100644 --- a/lib/tests/streamlit/file_uploader_test.py +++ b/lib/tests/streamlit/file_uploader_test.py @@ -125,7 +125,7 @@ def test_unique_uploaded_file_instance(self, get_file_recs_patch): file1: UploadedFile = st.file_uploader("a", accept_multiple_files=False) file2: UploadedFile = st.file_uploader("b", accept_multiple_files=False) - self.assertNotEqual(file1, file2) + self.assertNotEqual(id(file1), id(file2)) # Seeking in one instance should not impact the position in the other. file1.seek(2) From d339a5c1ea403fdd262f6e3cf4f46ab6796135d1 Mon Sep 17 00:00:00 2001 From: kajarenc Date: Sat, 22 Jan 2022 22:53:36 +0400 Subject: [PATCH 02/10] fix comment after review, raise NotImplemented to be semantically correct --- lib/streamlit/uploaded_file_manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/streamlit/uploaded_file_manager.py b/lib/streamlit/uploaded_file_manager.py index 386541354ff4..55b4ae768201 100644 --- a/lib/streamlit/uploaded_file_manager.py +++ b/lib/streamlit/uploaded_file_manager.py @@ -50,10 +50,10 @@ def __init__(self, record: UploadedFileRec): self.type = record.type self.size = len(record.data) - def __eq__(self, other): - if other is not None: - return self.id == other.id - return False + def __eq__(self, other: object) -> bool: + if not isinstance(other, UploadedFile): + return NotImplemented + return self.id == other.id def __repr__(self) -> str: return util.repr_(self) From 5cde03a0406c71437d50993a123cc78017bfdb35 Mon Sep 17 00:00:00 2001 From: kajarenc Date: Sat, 22 Jan 2022 23:08:18 +0400 Subject: [PATCH 03/10] Simplify test_file_uploader_serde test, since now we can use `check_roundtrip` here --- lib/tests/streamlit/state/session_state_test.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/lib/tests/streamlit/state/session_state_test.py b/lib/tests/streamlit/state/session_state_test.py index 6633bb50fe06..7b2e40acf606 100644 --- a/lib/tests/streamlit/state/session_state_test.py +++ b/lib/tests/streamlit/state/session_state_test.py @@ -305,22 +305,7 @@ def test_file_uploader_serde(self, get_file_recs_patch): get_file_recs_patch.return_value = file_recs uploaded_file = st.file_uploader("file_uploader", key="file_uploader") - - # We can't use check_roundtrip here as the return_value of a - # file_uploader widget isn't a primitive value, so comparing them - # using == checks for reference equality. - session_state = get_session_state() - metadata = session_state.get_metadata_by_key("file_uploader") - serializer = metadata.serializer - deserializer = metadata.deserializer - - file_after_serde = deserializer(serializer(uploaded_file), "") - - assert uploaded_file.id == file_after_serde.id - assert uploaded_file.name == file_after_serde.name - assert uploaded_file.type == file_after_serde.type - assert uploaded_file.size == file_after_serde.size - assert uploaded_file.read() == file_after_serde.read() + check_roundtrip("file_uploader", uploaded_file) def test_multiselect_serde(self): multiselect = st.multiselect( From 6ab523e60c81554548b6c44d3dc1e08b9f81bc3c Mon Sep 17 00:00:00 2001 From: kajarenc Date: Sun, 23 Jan 2022 21:28:24 +0400 Subject: [PATCH 04/10] Add regression test --- e2e/scripts/st_file_uploader.py | 16 ++++++++++ e2e/specs/st_file_uploader.spec.js | 47 ++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/e2e/scripts/st_file_uploader.py b/e2e/scripts/st_file_uploader.py index 4c33bed48433..66d7ef94e0a1 100644 --- a/e2e/scripts/st_file_uploader.py +++ b/e2e/scripts/st_file_uploader.py @@ -56,3 +56,19 @@ st.text("No upload") else: st.text(form_file.read()) + +if not st.session_state.get("counter"): + st.session_state.counter = 0 + + +def file_uploader_on_change(): + st.session_state.counter += 1 + + +single_on_change = st.file_uploader( + "Drop a file:", + type=["txt"], + key="on_change_example", + on_change=file_uploader_on_change, +) +st.text(st.session_state.counter) diff --git a/e2e/specs/st_file_uploader.spec.js b/e2e/specs/st_file_uploader.spec.js index f60e8d659e2f..a6c8f4c38800 100644 --- a/e2e/specs/st_file_uploader.spec.js +++ b/e2e/specs/st_file_uploader.spec.js @@ -395,4 +395,51 @@ describe("st.file_uploader", () => { ); }); }); + + // regression test for https://github.com/streamlit/streamlit/issues/4256 bug + it("not calls callback if not changed", () => { + const fileName1 = "file1.txt"; + const uploaderIndex = 4; + + cy.fixture(fileName1).then(file1 => { + const files = [ + { fileContent: file1, fileName: fileName1, mimeType: "text/plain" } + ]; + + // Script contains counter variable stored in session_state with + // default value 0. We increment counter inside file_uploader callback + // Since callback did not called at this moment, counter value should + // be equal 0 + cyGetIndexed("[data-testid='stText']", uploaderIndex).should( + "contain.text", + "0" + ); + + // Uploading file, should invoke on_change call and counter increment + cyGetIndexed( + "[data-testid='stFileUploadDropzone']", + uploaderIndex + ).attachFile(files[0], { + force: true, + subjectType: "drag-n-drop", + events: ["dragenter", "drop"] + }); + + // Make sure callback called + cyGetIndexed("[data-testid='stText']", uploaderIndex).should( + "contain.text", + "1" + ); + + // On rerun, make sure callback is not called, since file not changed + cy.get("body").type("r"); + cy.wait(1000); + + // Counter should be still equal 1 + cyGetIndexed("[data-testid='stText']", uploaderIndex).should( + "contain.text", + "1" + ); + }); + }); }); From e339cc42357f82efa143f8ee2be778c1bc0ad585 Mon Sep 17 00:00:00 2001 From: kajarenc Date: Sun, 23 Jan 2022 22:03:09 +0400 Subject: [PATCH 05/10] fix after merge --- e2e/specs/st_file_uploader.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/e2e/specs/st_file_uploader.spec.js b/e2e/specs/st_file_uploader.spec.js index 70d082c009f5..e94a60172c91 100644 --- a/e2e/specs/st_file_uploader.spec.js +++ b/e2e/specs/st_file_uploader.spec.js @@ -408,13 +408,13 @@ describe("st.file_uploader", () => { // default value 0. We increment counter inside file_uploader callback // Since callback did not called at this moment, counter value should // be equal 0 - cyGetIndexed("[data-testid='stText']", uploaderIndex).should( + cy.getIndexed("[data-testid='stText']", uploaderIndex).should( "contain.text", "0" ); // Uploading file, should invoke on_change call and counter increment - cyGetIndexed( + cy.getIndexed( "[data-testid='stFileUploadDropzone']", uploaderIndex ).attachFile(files[0], { @@ -424,7 +424,7 @@ describe("st.file_uploader", () => { }); // Make sure callback called - cyGetIndexed("[data-testid='stText']", uploaderIndex).should( + cy.getIndexed("[data-testid='stText']", uploaderIndex).should( "contain.text", "1" ); @@ -434,7 +434,7 @@ describe("st.file_uploader", () => { cy.wait(1000); // Counter should be still equal 1 - cyGetIndexed("[data-testid='stText']", uploaderIndex).should( + cy.getIndexed("[data-testid='stText']", uploaderIndex).should( "contain.text", "1" ); From 80f88048622d68536a978a853beac29bfa1ceafd Mon Sep 17 00:00:00 2001 From: kajarenc Date: Tue, 25 Jan 2022 00:48:24 +0400 Subject: [PATCH 06/10] remove unused variable from test --- e2e/scripts/st_file_uploader.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e/scripts/st_file_uploader.py b/e2e/scripts/st_file_uploader.py index 68738be40857..a5dd6231da45 100644 --- a/e2e/scripts/st_file_uploader.py +++ b/e2e/scripts/st_file_uploader.py @@ -65,10 +65,11 @@ def file_uploader_on_change(): st.session_state.counter += 1 -single_on_change = st.file_uploader( +st.file_uploader( "Drop a file:", type=["txt"], key="on_change_example", on_change=file_uploader_on_change, ) + st.text(st.session_state.counter) From aaa123fce397317601a70815954f9d010d33abe8 Mon Sep 17 00:00:00 2001 From: kajarenc Date: Tue, 25 Jan 2022 01:01:38 +0400 Subject: [PATCH 07/10] remove tests, check for test --- e2e/scripts/st_file_uploader.py | 17 ----------- e2e/specs/st_file_uploader.spec.js | 47 ------------------------------ 2 files changed, 64 deletions(-) diff --git a/e2e/scripts/st_file_uploader.py b/e2e/scripts/st_file_uploader.py index a5dd6231da45..1ecc534993b3 100644 --- a/e2e/scripts/st_file_uploader.py +++ b/e2e/scripts/st_file_uploader.py @@ -56,20 +56,3 @@ st.text("No upload") else: st.text(form_file.read()) - -if not st.session_state.get("counter"): - st.session_state.counter = 0 - - -def file_uploader_on_change(): - st.session_state.counter += 1 - - -st.file_uploader( - "Drop a file:", - type=["txt"], - key="on_change_example", - on_change=file_uploader_on_change, -) - -st.text(st.session_state.counter) diff --git a/e2e/specs/st_file_uploader.spec.js b/e2e/specs/st_file_uploader.spec.js index e94a60172c91..9194efc665ad 100644 --- a/e2e/specs/st_file_uploader.spec.js +++ b/e2e/specs/st_file_uploader.spec.js @@ -393,51 +393,4 @@ describe("st.file_uploader", () => { ); }); }); - - // regression test for https://github.com/streamlit/streamlit/issues/4256 bug - it("not calls callback if not changed", () => { - const fileName1 = "file1.txt"; - const uploaderIndex = 4; - - cy.fixture(fileName1).then(file1 => { - const files = [ - { fileContent: file1, fileName: fileName1, mimeType: "text/plain" } - ]; - - // Script contains counter variable stored in session_state with - // default value 0. We increment counter inside file_uploader callback - // Since callback did not called at this moment, counter value should - // be equal 0 - cy.getIndexed("[data-testid='stText']", uploaderIndex).should( - "contain.text", - "0" - ); - - // Uploading file, should invoke on_change call and counter increment - cy.getIndexed( - "[data-testid='stFileUploadDropzone']", - uploaderIndex - ).attachFile(files[0], { - force: true, - subjectType: "drag-n-drop", - events: ["dragenter", "drop"] - }); - - // Make sure callback called - cy.getIndexed("[data-testid='stText']", uploaderIndex).should( - "contain.text", - "1" - ); - - // On rerun, make sure callback is not called, since file not changed - cy.get("body").type("r"); - cy.wait(1000); - - // Counter should be still equal 1 - cy.getIndexed("[data-testid='stText']", uploaderIndex).should( - "contain.text", - "1" - ); - }); - }); }); From dedae3dcbe741a7b76e8aa2f258bdfe38c670b07 Mon Sep 17 00:00:00 2001 From: kajarenc Date: Tue, 25 Jan 2022 01:20:14 +0400 Subject: [PATCH 08/10] exclude latest pandas, check for tests --- lib/Pipfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Pipfile b/lib/Pipfile index c3212edc2f2e..a24afdb910e9 100644 --- a/lib/Pipfile +++ b/lib/Pipfile @@ -44,7 +44,7 @@ cachetools = ">=4.0" click = ">=7.0" numpy = "*" packaging = "*" -pandas = ">=0.21.0" +pandas = ">=0.21.0 !=1.4.0" pillow = ">=6.2.0" # protobuf version 3.11 is incompatible, see https://github.com/streamlit/streamlit/issues/2234 protobuf = ">=3.6.0, !=3.11" From 1e273630a1ebd0766550d7c656eea06404f98e0f Mon Sep 17 00:00:00 2001 From: kajarenc Date: Tue, 25 Jan 2022 01:22:13 +0400 Subject: [PATCH 09/10] try to fix failing tests --- lib/Pipfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Pipfile b/lib/Pipfile index a24afdb910e9..e37f0ca8142a 100644 --- a/lib/Pipfile +++ b/lib/Pipfile @@ -44,7 +44,7 @@ cachetools = ">=4.0" click = ">=7.0" numpy = "*" packaging = "*" -pandas = ">=0.21.0 !=1.4.0" +pandas = ">=0.21.0, !=1.4.0" pillow = ">=6.2.0" # protobuf version 3.11 is incompatible, see https://github.com/streamlit/streamlit/issues/2234 protobuf = ">=3.6.0, !=3.11" From 7eceb95e74002dd071bd37be02facc55626b27f5 Mon Sep 17 00:00:00 2001 From: kajarenc Date: Tue, 25 Jan 2022 02:22:56 +0400 Subject: [PATCH 10/10] Remove latest Pandas version exclusion --- lib/Pipfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Pipfile b/lib/Pipfile index e37f0ca8142a..c3212edc2f2e 100644 --- a/lib/Pipfile +++ b/lib/Pipfile @@ -44,7 +44,7 @@ cachetools = ">=4.0" click = ">=7.0" numpy = "*" packaging = "*" -pandas = ">=0.21.0, !=1.4.0" +pandas = ">=0.21.0" pillow = ">=6.2.0" # protobuf version 3.11 is incompatible, see https://github.com/streamlit/streamlit/issues/2234 protobuf = ">=3.6.0, !=3.11"