From 522e9ca084912874fe024a7b015e69d6ab1831fa Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Fri, 10 Jan 2025 10:07:28 -0500 Subject: [PATCH 1/5] UI structure for info box --- dp_wizard/app/components/outputs.py | 4 ++++ dp_wizard/app/dataset_panel.py | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/dp_wizard/app/components/outputs.py b/dp_wizard/app/components/outputs.py index cb0f4b3..e63eac6 100644 --- a/dp_wizard/app/components/outputs.py +++ b/dp_wizard/app/components/outputs.py @@ -22,3 +22,7 @@ def demo_tooltip(is_demo: bool, text: str): # pragma: no cover def hide_if(condition: bool, el): # pragma: no cover display = "none" if condition else "block" return ui.div(el, style=f"display: {display};") + + +def info_box(text): # pragma: no cover + return ui.div(text, class_="alert alert-info", role="alert") diff --git a/dp_wizard/app/dataset_panel.py b/dp_wizard/app/dataset_panel.py index cdfc9ae..3412a90 100644 --- a/dp_wizard/app/dataset_panel.py +++ b/dp_wizard/app/dataset_panel.py @@ -3,7 +3,7 @@ from shiny import ui, reactive, render, Inputs, Outputs, Session from dp_wizard.utils.argparse_helpers import get_cli_info -from dp_wizard.app.components.outputs import output_code_sample, demo_tooltip +from dp_wizard.app.components.outputs import output_code_sample, demo_tooltip, info_box from dp_wizard.utils.code_generators import make_privacy_unit_block @@ -25,11 +25,14 @@ def dataset_ui(): "How many rows of the CSV can one individual contribute to? " 'This is the "unit of privacy" which will be protected.' ), - ui.input_numeric( - "contributions", - ["Contributions", ui.output_ui("contributions_demo_tooltip_ui")], - cli_info.contributions, - min=1, + ui.row( + ui.input_numeric( + "contributions", + ["Contributions", ui.output_ui("contributions_demo_tooltip_ui")], + cli_info.contributions, + min=1, + ), + ui.output_ui("contributions_validation_ui"), ), ui.output_ui("python_tooltip_ui"), output_code_sample("Unit of Privacy", "unit_of_privacy_python"), @@ -80,6 +83,10 @@ def contributions_demo_tooltip_ui(): f"can occur at most {contributions()} times in the dataset. ", ) + @render.ui + def contributions_validation_ui(): + return info_box(ui.markdown("TODO: conditional warning here.")) + @render.ui def python_tooltip_ui(): return demo_tooltip( From cfb5dcd28e33c83667d971ad071fa509065339f9 Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Fri, 10 Jan 2025 10:26:27 -0500 Subject: [PATCH 2/5] conditional logic --- dp_wizard/app/components/outputs.py | 4 ++-- dp_wizard/app/dataset_panel.py | 13 +++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/dp_wizard/app/components/outputs.py b/dp_wizard/app/components/outputs.py index e63eac6..bf26475 100644 --- a/dp_wizard/app/components/outputs.py +++ b/dp_wizard/app/components/outputs.py @@ -24,5 +24,5 @@ def hide_if(condition: bool, el): # pragma: no cover return ui.div(el, style=f"display: {display};") -def info_box(text): # pragma: no cover - return ui.div(text, class_="alert alert-info", role="alert") +def info_box(content): # pragma: no cover + return ui.div(content, class_="alert alert-info", role="alert") diff --git a/dp_wizard/app/dataset_panel.py b/dp_wizard/app/dataset_panel.py index 3412a90..c6aad51 100644 --- a/dp_wizard/app/dataset_panel.py +++ b/dp_wizard/app/dataset_panel.py @@ -3,7 +3,12 @@ from shiny import ui, reactive, render, Inputs, Outputs, Session from dp_wizard.utils.argparse_helpers import get_cli_info -from dp_wizard.app.components.outputs import output_code_sample, demo_tooltip, info_box +from dp_wizard.app.components.outputs import ( + output_code_sample, + demo_tooltip, + info_box, + hide_if, +) from dp_wizard.utils.code_generators import make_privacy_unit_block @@ -85,7 +90,11 @@ def contributions_demo_tooltip_ui(): @render.ui def contributions_validation_ui(): - return info_box(ui.markdown("TODO: conditional warning here.")) + contributions = input.contributions() + return hide_if( + isinstance(contributions, int) and contributions >= 1, + info_box(ui.markdown("Contributions must be 1 or greater.")), + ) @render.ui def python_tooltip_ui(): From cc29e39b01c95ab5e36f48569126c78acc073781 Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Fri, 10 Jan 2025 10:34:01 -0500 Subject: [PATCH 3/5] factor out reactive calc --- dp_wizard/app/dataset_panel.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dp_wizard/app/dataset_panel.py b/dp_wizard/app/dataset_panel.py index c6aad51..a02bc60 100644 --- a/dp_wizard/app/dataset_panel.py +++ b/dp_wizard/app/dataset_panel.py @@ -88,11 +88,15 @@ def contributions_demo_tooltip_ui(): f"can occur at most {contributions()} times in the dataset. ", ) + @reactive.calc + def contributions_valid(): + contributions = input.contributions() + return isinstance(contributions, int) and contributions >= 1 + @render.ui def contributions_validation_ui(): - contributions = input.contributions() return hide_if( - isinstance(contributions, int) and contributions >= 1, + contributions_valid(), info_box(ui.markdown("Contributions must be 1 or greater.")), ) @@ -111,7 +115,7 @@ def define_analysis_button_ui(): button = ui.input_action_button( "go_to_analysis", "Define analysis", disabled=not button_enabled() ) - if button_enabled(): + if button_enabled() and contributions_valid(): return button return [ button, From 872de3c8aca54d2bbc6de2a240711ba44743814b Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Fri, 10 Jan 2025 10:58:54 -0500 Subject: [PATCH 4/5] test validation --- tests/test_app.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_app.py b/tests/test_app.py index 283072c..5b23884 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -64,6 +64,18 @@ def expect_no_error(): # Now upload: csv_path = Path(__file__).parent / "fixtures" / "fake.csv" page.get_by_label("Choose CSV file").set_input_files(csv_path.resolve()) + + # Check validation of contributions: + # Playwright itself won't let us fill non-numbers in this field. + # "assert define_analysis_button.is_enabled()" has spurious errors. + page.get_by_label("Contributions").fill("0") + expect_visible("Contributions must be 1 or greater") + expect_visible("Choose CSV and Contributions before proceeding") + + page.get_by_label("Contributions").fill("42") + expect_not_visible("Contributions must be 1 or greater") + expect_not_visible("Choose CSV and Contributions before proceeding") + expect_no_error() # -- Define analysis -- From d3405dec2836eceda40201c8c43974ac713b5e27 Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Wed, 15 Jan 2025 14:14:20 -0500 Subject: [PATCH 5/5] link to issue for flaky test --- tests/test_app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_app.py b/tests/test_app.py index 5b23884..1215c3d 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -68,6 +68,7 @@ def expect_no_error(): # Check validation of contributions: # Playwright itself won't let us fill non-numbers in this field. # "assert define_analysis_button.is_enabled()" has spurious errors. + # https://github.com/opendp/dp-wizard/issues/221 page.get_by_label("Contributions").fill("0") expect_visible("Contributions must be 1 or greater") expect_visible("Choose CSV and Contributions before proceeding")