[BUGFIX] Data Assistant plotting with zero expectations produced #5934

NathanFarmer · 2022-09-06T15:17:05Z

Changes proposed in this pull request:

GREAT-761/GREAT-1233
Fix bug that raises error if zero expectations are produced by the Data Assistant

Definition of Done

My code follows the Great Expectations style guide
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added unit tests where applicable and made sure that new and existing tests are passing.
I have run any local integration tests and made sure that nothing is broken.

…ferredAssetSqlDataConnector" as well as update of "SimpleSqlalchemyDatasource" -- with numerous additional unit tests.

…exsherstinsky/sql_connector_certification-2022_08_23-221

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

…T-200/GREAT-556/alexsherstinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

…EAT-1218/alexsherstinsky/stats_data_assistant_spark_environments_recommendations-2022_09_01-226

NathanFarmer · 2022-09-06T15:44:07Z

great_expectations/rule_based_profiler/data_assistant_result/data_assistant_result.py

-            display_chart_dict=widgets.fixed(display_chart_dict),
-            chart_title=dropdown_selection,
-        )
+        if len(chart_titles) > 0:


This block is unchanged except to make sure we have charts to display.

NathanFarmer · 2022-09-06T15:46:17Z

great_expectations/rule_based_profiler/data_assistant_result/data_assistant_result.py

-                    ]
-                else:
-                    df[kwarg_name] = expectation_configuration.kwargs[kwarg_name]
+            if expectation_configuration is not None:


Since we are in diagnostic mode, if there is no expectation configuration associated with this metric, return None.

Just a suggestion - could we return an empty dataframe instead of None to make the return type consistent?

…expectations

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

…expectations

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

…expectations

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

…expectations

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

anthonyburdi

Looks good to me! Two minor non blocking questions/suggestions.

anthonyburdi · 2022-09-07T13:45:38Z

great_expectations/rule_based_profiler/data_assistant_result/data_assistant_result.py

-                    ]
-                else:
-                    df[kwarg_name] = expectation_configuration.kwargs[kwarg_name]
+            if expectation_configuration is not None:


Just a suggestion - could we return an empty dataframe instead of None to make the return type consistent?

tests/rule_based_profiler/data_assistant/test_onboarding_data_assistant.py

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

…red-sql-multi-batch-notebook * develop: [BUGFIX] prefix and suffix asset names are only relevant for InferredSqlAlchemyDataConnector (#5950) Add a dev-tools requirements option (#5944) [BUGFIX] Data Assistant plotting with zero expectations produced (#5934) [MAINTENANCE] Unit tests for `ConfigurationStore` (#5948) [BUGFIX] Don't include abstract Expectation classes in _retrieve_expectations_from_module (#5947) [BUGFIX] Making an all-NULL column handling in RuleBasedProfiler more robust (#5937)

…r-usage-stats-opt * develop: (62 commits) [MAINTENANCE] Bump `Marshmallow` upper bound to work with Airflow operator (#5952) [MAINTENANCE] Add x-fails to flaky Cloud tests for purposes of 0.15.22 (#5964) [BUGFIX] Prevent "division by zero" errors in Rule-Based Profiler calculations when Batch has zero rows (#5960) [FEATURE] Improve slack error condition (#5818) [MAINTENANCE] More unit tests for `Stores` (#5953) [MAINTENANCE] Unit tests for `ValidationGraph` and related classes (#5954) [MAINTENANCE] Run spark and onboarding data assistant test in their own jobs. (#5951) [BUGFIX] prefix and suffix asset names are only relevant for InferredSqlAlchemyDataConnector (#5950) Add a dev-tools requirements option (#5944) [BUGFIX] Data Assistant plotting with zero expectations produced (#5934) [MAINTENANCE] Unit tests for `ConfigurationStore` (#5948) [BUGFIX] Don't include abstract Expectation classes in _retrieve_expectations_from_module (#5947) [BUGFIX] Making an all-NULL column handling in RuleBasedProfiler more robust (#5937) [MAINTENANCE] Run comprehensive tests in a random order (#5942) [MAINTENANCE] Update to OnboardingDataAssistant Notebook - Sql (#5939) [MAINTENANCE] Add missing import for ConfigurationIdentifier (#5943) [MAINTENANCE] Mark tests within `tests/rule_based_profiler` (#5930) [MAINTENANCE] Move `Store` test utils from source code to tests (#5932) [FEATURE] DataAssistants Example Notebook - Spark (#5919) [MAINTENANCE] Reset globals modified in tests (#5936) ...

Alex Sherstinsky added 30 commits August 25, 2022 14:20

Update and certification of "ConfiguredAssetSqlDataConnector" and "In…

de1aa29

…ferredAssetSqlDataConnector" as well as update of "SimpleSqlalchemyDatasource" -- with numerous additional unit tests.

Merge branch 'develop' into pre_pr-prototype/maintenance/GREAT-556/al…

e126934

…exsherstinsky/sql_connector_certification-2022_08_23-221

clean up

e57159f

merge

e343090

type hint

597a4d2

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

38f1afd

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

clean up

a87e38e

clean up

32e21c8

merge

ff3c608

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

285939e

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

missing configs added

05f9cd9

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

b03ee0d

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

clean up

fa241b1

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

d0bc991

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

29b9620

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

061643d

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

dfd648f

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

b89771a

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

1cc0cff

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

clean up

67ff31c

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

9e35f85

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

Merge remote-tracking branch 'upstream/develop' into maintenance/GREA…

e241f6f

…T-200/GREAT-556/alexsherstinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

clean up

f9f5b90

clean up unused imports

b071766

Merge branch 'develop' into maintenance/GREAT-200/GREAT-556/alexshers…

0c3d4ca

…tinsky/datasource/data_connector/asset/update_to_parity_and_certify_configured_and_inferred_asset_sql_data_connectors_and_simple_sqlalchemy_datasource-2022_08_25-221

Merge branch 'develop' into pre_pr-prototype/maintenance/GREAT-761/GR…

d48ba4b

…EAT-1218/alexsherstinsky/stats_data_assistant_spark_environments_recommendations-2022_09_01-226

statistics data assistant

1e51c91

naming

a0b4323

init

ed67b3d

init

f44b654

NathanFarmer commented Sep 6, 2022

View reviewed changes

Nathan Farmer and others added 2 commits September 6, 2022 11:51

Remove duplicated logic

0d70238

Merge branch 'develop' into bugfix/data-assistant-plotting-with-zero-…

8b71357

…expectations

NathanFarmer requested a review from a team September 6, 2022 15:57

Nathan Farmer and others added 14 commits September 6, 2022 13:32

Bug resurfaced after testing with OnboardingDataAssistant

1c8f563

Merge branch 'bugfix/data-assistant-plotting-with-zero-expectations' …

5be7da7

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

Merge branch 'develop' into bugfix/data-assistant-plotting-with-zero-…

82fad11

…expectations

Lint

aeae9a0

Merge branch 'bugfix/data-assistant-plotting-with-zero-expectations' …

6e3c314

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

Merge branch 'develop' into bugfix/data-assistant-plotting-with-zero-…

8e30e3f

…expectations

Merge branch 'develop' into bugfix/data-assistant-plotting-with-zero-…

ad9b25e

…expectations

Merge branch 'develop' into bugfix/data-assistant-plotting-with-zero-…

5bc8b1d

…expectations

Add previously failing test to prevent regresesion

2dbb520

Merge branch 'bugfix/data-assistant-plotting-with-zero-expectations' …

0be415a

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

Merge branch 'develop' into bugfix/data-assistant-plotting-with-zero-…

57c765b

…expectations

metric_df can be None and therefore should be treated that way

49a3378

Merge branch 'bugfix/data-assistant-plotting-with-zero-expectations' …

1ee812c

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

One more condition after performing UAT

9566034

anthonyburdi approved these changes Sep 7, 2022

View reviewed changes

Nathan Farmer added 2 commits September 7, 2022 10:07

Add new test

4775419

Keep return types consistent for _create_df_for_charting

489883d

NathanFarmer enabled auto-merge (squash) September 7, 2022 14:19

github-actions bot and others added 3 commits September 7, 2022 14:30

Merge develop into bugfix/data-assistant-plotting-with-zero-expectations

7518a08

Remove test mistakenly added to this branch

f0e561e

Merge branch 'bugfix/data-assistant-plotting-with-zero-expectations' …

2ac4089

…of github.com:great-expectations/great_expectations into bugfix/data-assistant-plotting-with-zero-expectations

NathanFarmer merged commit d12e9af into develop Sep 7, 2022

NathanFarmer deleted the bugfix/data-assistant-plotting-with-zero-expectations branch September 7, 2022 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUGFIX] Data Assistant plotting with zero expectations produced #5934

[BUGFIX] Data Assistant plotting with zero expectations produced #5934

NathanFarmer commented Sep 6, 2022 •

edited

NathanFarmer Sep 6, 2022

NathanFarmer Sep 6, 2022

anthonyburdi Sep 7, 2022

anthonyburdi left a comment

anthonyburdi Sep 7, 2022

[BUGFIX] Data Assistant plotting with zero expectations produced #5934

[BUGFIX] Data Assistant plotting with zero expectations produced #5934

Conversation

NathanFarmer commented Sep 6, 2022 • edited

Definition of Done

NathanFarmer Sep 6, 2022

Choose a reason for hiding this comment

NathanFarmer Sep 6, 2022

Choose a reason for hiding this comment

anthonyburdi Sep 7, 2022

Choose a reason for hiding this comment

anthonyburdi left a comment

Choose a reason for hiding this comment

anthonyburdi Sep 7, 2022

Choose a reason for hiding this comment

NathanFarmer commented Sep 6, 2022 •

edited