From b2e50b473a4e6d20f40196969c40effc07caa77e Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Thu, 7 Nov 2024 13:52:21 -0500 Subject: [PATCH 1/4] Do not label every bin (#148) * perhaps gratuitous? * more idiomatic use of major and minor bins --- dp_creator_ii/app/components/plots.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dp_creator_ii/app/components/plots.py b/dp_creator_ii/app/components/plots.py index b1df40d..1e33a28 100644 --- a/dp_creator_ii/app/components/plots.py +++ b/dp_creator_ii/app/components/plots.py @@ -15,9 +15,14 @@ def _df_to_columns(df): def plot_histogram(histogram_df, error, cutoff): # pragma: no cover - labels, values = _df_to_columns(histogram_df) + bins, values = _df_to_columns(histogram_df) + mod = (len(bins) // 12) + 1 + majors = [label for i, label in enumerate(bins) if i % mod == 0] + minors = [label for i, label in enumerate(bins) if i % mod != 0] _figure, axes = plt.subplots() bar_colors = ["blue" if v > cutoff else "lightblue" for v in values] - axes.bar(labels, values, color=bar_colors, yerr=error) + axes.bar(bins, values, color=bar_colors, yerr=error) + axes.set_xticks(majors, majors) + axes.set_xticks(minors, ["" for _ in minors], minor=True) axes.axhline(cutoff, color="lightgrey", zorder=-1) # TODO: Since this seems to return None, how does the information flow? From 5a0f3797ff68c8a4d269481c8e59b1f0964e7889 Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Thu, 7 Nov 2024 13:52:39 -0500 Subject: [PATCH 2/4] rename tests to match the files they test (#117) * rename tests to match the files they test * make tests/utils... worry a little that it could be misread as utils for tests * fix paths --- .../test_argparse_helpers.py} | 9 ++++++--- tests/{ => utils}/test_converters.py | 5 +++-- tests/{test_csv.py => utils/test_csv_helper.py} | 0 tests/{ => utils}/test_misc.py | 0 tests/{test_template.py => utils/test_templates.py} | 5 ++--- 5 files changed, 11 insertions(+), 8 deletions(-) rename tests/{test_arg_parser.py => utils/test_argparse_helpers.py} (79%) rename tests/{ => utils}/test_converters.py (93%) rename tests/{test_csv.py => utils/test_csv_helper.py} (100%) rename tests/{ => utils}/test_misc.py (100%) rename tests/{test_template.py => utils/test_templates.py} (95%) diff --git a/tests/test_arg_parser.py b/tests/utils/test_argparse_helpers.py similarity index 79% rename from tests/test_arg_parser.py rename to tests/utils/test_argparse_helpers.py index d20c850..e56b2b8 100644 --- a/tests/test_arg_parser.py +++ b/tests/utils/test_argparse_helpers.py @@ -6,6 +6,9 @@ from dp_creator_ii.utils.argparse_helpers import _get_arg_parser, _existing_csv_type +fixtures_path = Path(__file__).parent.parent / "fixtures" + + def test_help(): help = ( _get_arg_parser() @@ -19,7 +22,7 @@ def test_help(): ) print(help) - readme_md = (Path(__file__).parent.parent / "README.md").read_text() + readme_md = (Path(__file__).parent.parent.parent / "README.md").read_text() assert help in readme_md @@ -30,9 +33,9 @@ def test_arg_validation_no_file(): def test_arg_validation_not_csv(): with pytest.raises(ArgumentTypeError, match='Must have ".csv" extension:'): - _existing_csv_type(Path(__file__).parent / "fixtures" / "fake.ipynb") + _existing_csv_type(fixtures_path / "fake.ipynb") def test_arg_validation_works(): - path = _existing_csv_type(Path(__file__).parent / "fixtures" / "fake.csv") + path = _existing_csv_type(fixtures_path / "fake.csv") assert path.name == "fake.csv" diff --git a/tests/test_converters.py b/tests/utils/test_converters.py similarity index 93% rename from tests/test_converters.py rename to tests/utils/test_converters.py index 4f1a16c..9e4c34e 100644 --- a/tests/test_converters.py +++ b/tests/utils/test_converters.py @@ -5,6 +5,9 @@ from dp_creator_ii.utils.converters import convert_py_to_nb +fixtures_path = Path(__file__).parent.parent / "fixtures" + + def norm_nb(nb_str): normed_nb_str = nb_str normed_nb_str = re.sub(r'"id": "[^"]+"', '"id": "12345678"', normed_nb_str) @@ -21,7 +24,6 @@ def norm_nb(nb_str): def test_convert_py_to_nb(): - fixtures_path = Path(__file__).parent / "fixtures" python_str = (fixtures_path / "fake.py").read_text() actual_nb_str = convert_py_to_nb(python_str) expected_nb_str = (fixtures_path / "fake.ipynb").read_text() @@ -32,7 +34,6 @@ def test_convert_py_to_nb(): def test_convert_py_to_nb_execute(): - fixtures_path = Path(__file__).parent / "fixtures" python_str = (fixtures_path / "fake.py").read_text() actual_nb_str = convert_py_to_nb(python_str, execute=True) expected_nb_str = (fixtures_path / "fake-executed.ipynb").read_text() diff --git a/tests/test_csv.py b/tests/utils/test_csv_helper.py similarity index 100% rename from tests/test_csv.py rename to tests/utils/test_csv_helper.py diff --git a/tests/test_misc.py b/tests/utils/test_misc.py similarity index 100% rename from tests/test_misc.py rename to tests/utils/test_misc.py diff --git a/tests/test_template.py b/tests/utils/test_templates.py similarity index 95% rename from tests/test_template.py rename to tests/utils/test_templates.py index e051c57..b65c898 100644 --- a/tests/test_template.py +++ b/tests/utils/test_templates.py @@ -7,6 +7,7 @@ from dp_creator_ii.utils.templates import _Template, make_notebook_py, make_script_py +fixtures_path = Path(__file__).parent.parent / "fixtures" fake_csv = "tests/fixtures/fake.csv" @@ -113,9 +114,7 @@ def clear_empty_lines(text): # Cleanup whitespace after indenting blocks return re.sub(r"^\s+$", "", text, flags=re.MULTILINE).strip() - expected_script = ( - Path(__file__).parent / "fixtures" / "expected-script.py" - ).read_text() + expected_script = (fixtures_path / "expected-script.py").read_text() assert clear_empty_lines(script) == clear_empty_lines(expected_script) with NamedTemporaryFile(mode="w") as fp: From 2f08f376b2bdb5b2191e2f74165de591e04a27a5 Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Thu, 7 Nov 2024 14:47:25 -0500 Subject: [PATCH 3/4] set bottom of axis (#145) --- dp_creator_ii/app/components/plots.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dp_creator_ii/app/components/plots.py b/dp_creator_ii/app/components/plots.py index 1e33a28..bd017b6 100644 --- a/dp_creator_ii/app/components/plots.py +++ b/dp_creator_ii/app/components/plots.py @@ -25,4 +25,5 @@ def plot_histogram(histogram_df, error, cutoff): # pragma: no cover axes.set_xticks(majors, majors) axes.set_xticks(minors, ["" for _ in minors], minor=True) axes.axhline(cutoff, color="lightgrey", zorder=-1) + axes.set_ylim(bottom=0) # TODO: Since this seems to return None, how does the information flow? From 26a3115b1142b89dc55e4f7c0b7c1758fa828808 Mon Sep 17 00:00:00 2001 From: Chuck McCallum Date: Thu, 7 Nov 2024 16:40:13 -0500 Subject: [PATCH 4/4] Two column layout: Graph expands to fit (#137) * two column layout * narrow controls * smaller plot * Annie prefers wider margin --- WHAT-WE-LEARNED.md | 4 ++ dp_creator_ii/app/components/column_module.py | 60 ++++++++++++------- dp_creator_ii/app/css/styles.css | 2 +- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/WHAT-WE-LEARNED.md b/WHAT-WE-LEARNED.md index 9045739..cbddfaa 100644 --- a/WHAT-WE-LEARNED.md +++ b/WHAT-WE-LEARNED.md @@ -62,3 +62,7 @@ I've had to tweak the CSS a few times: The different flavors of "Shiny" are a bit of nuissance when trying to find examples. The maturity of Shiny for R means that the vast majority of the examples are for R, even with Python in the search. It would be nice if the docs site remembered that I only want to look at docs for Core. + +## Shiny docs could have better formatting + +- https://shiny.posit.co/py/api/core/ui.layout_columns.html: bullet list not rendered correctly. diff --git a/dp_creator_ii/app/components/column_module.py b/dp_creator_ii/app/components/column_module.py index f3f41b8..7bc95d8 100644 --- a/dp_creator_ii/app/components/column_module.py +++ b/dp_creator_ii/app/components/column_module.py @@ -10,28 +10,44 @@ @module.ui def column_ui(): # pragma: no cover - return [ - ui.input_numeric("min", "Min", 0), - ui.input_numeric("max", "Max", 10), - ui.input_numeric("bins", "Bins", 10), - ui.input_select( - "weight", - "Weight", - choices={ - 1: "Less accurate", - 2: "Default", - 4: "More accurate", - }, - selected=2, - ), - output_code_sample("Column Definition", "column_code"), - ui.markdown( - "This simulation assumes a normal distribution " - "between the specified min and max. " - "Your data file has not been read except to determine the columns." - ), - ui.output_plot("column_plot"), - ] + width = "10em" # Just wide enough so the text isn't trucated. + return ui.layout_columns( + [ + ui.input_numeric("min", "Min", 0, width=width), + ui.input_numeric("max", "Max", 10, width=width), + ui.input_numeric("bins", "Bins", 10, width=width), + ui.input_select( + "weight", + "Weight", + choices={ + 1: "Less accurate", + 2: "Default", + 4: "More accurate", + }, + selected=2, + width=width, + ), + ], + [ + # TODO: This doesn't need to be repeated: could just go once at the top. + # https://github.com/opendp/dp-creator-ii/issues/138 + ui.markdown( + "This simulation assumes a normal distribution " + "between the specified min and max. " + "Your data file has not been read except to determine the columns." + ), + ui.output_plot("column_plot", height="300px"), + # Make plot smaller than default: about the same size as the other column. + output_code_sample("Column Definition", "column_code"), + ], + col_widths={ + # Controls stay roughly a constant width; + # Graph expands to fill space. + "sm": (4, 8), + "md": (3, 9), + "lg": (2, 10), + }, + ) @module.server diff --git a/dp_creator_ii/app/css/styles.css b/dp_creator_ii/app/css/styles.css index 4749715..a624854 100644 --- a/dp_creator_ii/app/css/styles.css +++ b/dp_creator_ii/app/css/styles.css @@ -1,5 +1,5 @@ body { - margin: 1em; + margin: 2em; } #top_level_nav {