Implemented options for `num_quantiles` feature in `NumericStatsMixin` #896

clee1152 · 2023-06-22T13:50:15Z

Original issue here.

* memory optimization preset ttrying again ttrying again 3 ttrying again 4 accidentally pushed my updated makefile * Wrote catch for invalid presets, wrote test for catch for invalid presets, debugged new optimization preset * Forgot to run pre-commit, fixed those issues * black doing weird things * made preset validation more maintainable by moving it to the constructor and getting rid of preset list

* RowStatisticsOptions: Add null row count Added null_row_count as an option in RowStatisticsOptions. It toggles the functionality for row_has_null_ratio and row_is_null_ratio in _update_row_statistics. * Unit test for RowStatisticOptions: * Black formatting * RowStatisticsOptions: Add null row count Added null_row_count as an option in RowStatisticsOptions. It toggles the functionality for row_has_null_ratio and row_is_null_ratio in _update_row_statistics. * Unit test for RowStatisticOptions: * Black formatting * added a unit test for RowStatisticsOptions * Deleted test cases that were written in the wrong file * updated testing for null_count toggle in _update_row_statistics * removed the RowStatisticsOptions from test_profiler_options imports * add line * Created toggle option for null_count * RowStatisticsOptions: Add implementation * Revert "RowStatisticsOptions: Add implementation" This reverts commit 2da6a93. * RowStatsticsOptions: Create option * fixed pre-commit error * Update dataprofiler/profilers/profiler_options.py Co-authored-by: Taylor Turner <[email protected]> * Update dataprofiler/profilers/profiler_options.py Co-authored-by: Taylor Turner <[email protected]> * fixed documentation --------- Co-authored-by: Taylor Turner <[email protected]>

* memory optimization preset ttrying again ttrying again 3 ttrying again 4 accidentally pushed my updated makefile * trying * trying * black doing weird things * trying * made preset validation more maintainable by moving it to the constructor and getting rid of preset list * Update to open-source in prep for wrapper changes for mem op preset * updated preset toggles and preset name (mem op -> large data) * updated tests to match * continued name and test and toggle updates * fix comments

* Implementing option * Implementing option * took out redundant if statement. added test case for when null_count is disabled. * attempt to check for conflicts between profile merges * added test to check if two profilers have null_count enabled before merging them together * fixed typo and added a trycatch to prevent failing test * No mocks needed. Fixed assertRaisesRegex error * Changed variables names and added a new test to check for check the null_count when null_count is disabled. * Changed name of test, moved tests to TestStructuredProfilerRowStatistics. Fixed position of if statement to prevent unnecessary code from running. * added null_count test cases * fixed indentation mistake * fixed typo * removed a useless commented a line * Updated test name

CLAassistant · 2023-06-22T13:50:20Z

All committers have signed the CLA.

dataprofiler/profilers/numerical_column_stats.py

dataprofiler/profilers/profiler_options.py

taylorfturner

needs tests for these edits
also one slight change not to return in __calculations

clee1152 · 2023-06-23T17:17:46Z

Changes:

Added tests in test_numerical_options.py for num_quantiles
removed num_quantiles return in calculations

dataprofiler/profilers/profiler_options.py

clee1152 · 2023-06-27T20:51:20Z

Fixed num_quantiles not passing the tests in the IntColumns, FloatColumns, TextColumns, and NumericStatsMixin tests.
Just need to implement test_num_quantiles_option

taylorfturner · 2023-06-27T22:19:47Z

@clee1152 you also need to add scipy>=1.4.1,<1.11.0 on L 13 in your requirements.txt file ... that will resolve the graph_profile testing errors

clee1152 · 2023-06-28T13:52:03Z

Finished writing tests for num_quantiles feature. Tests in test_num_quantiles_option.py.

dataprofiler/profilers/profiler_options.py

dataprofiler/tests/profilers/profiler_options/test_num_quantiles_option.py

Co-authored-by: Taylor Turner <[email protected]>

…es_option.py Co-authored-by: Taylor Turner <[email protected]>

JGSweets · 2023-07-05T14:25:35Z

dataprofiler/profilers/profiler_options.py

@@ -269,6 +269,40 @@ def _validate_helper(self, variable_path: str = "ModeOption") -> list[str]:
        return errors


+class NumQuantilesOption(BooleanOption):


I think we call this QuantilesOption instead? That way it is extensible to more than just the number of quantiles.

Yeah I agree -- more extensible for future development

JGSweets · 2023-07-05T14:30:16Z

dataprofiler/profilers/profiler_options.py

@@ -367,6 +404,7 @@ def __init__(self) -> None:
        self.median_abs_deviation = BooleanOption(is_enabled=True)
        self.num_zeros = BooleanOption(is_enabled=True)
        self.num_negatives = BooleanOption(is_enabled=True)
+        self.num_quantiles = NumQuantilesOption(is_enabled=True)


I'm wondering if this should be in NumericalOptions? We already have
self.histogram_and_quantiles = HistogramOption().

Should we rename HistogramOption and insert it in that class?

@clee1152 @taylorfturner , thoughts?

I think this makes sense. Does this mean we will not go with your previous suggestion of renaming NumQuantilesOption to QuantileOptions and instead just insert it into HistogramOption?

Fair Q. @taylorfturner may have more thoughts, but if we are keeping
histogram_and_quantiles
Then we should at least rename HistogramOption -> HistogramAndQuantilesOption
We could just move over the num_quantiles setting

That sounds good to me! Will see what @taylorfturner thinks, then implement.

Yes, @clee1152 and I just talked about this -- I'm game for self. histogram_and_quantiles = HistogramAndQuantilesOption() where self.num_quantiles is set in class HistogramAndQuantilesOption() @JGSweets

clee1152 · 2023-07-10T16:52:16Z

Migrated to new PR here.

lizlouise1335 and others added 4 commits June 13, 2023 16:50

clee1152 requested review from JGSweets, ksneab7, taylorfturner, micdavis and tyfarnan as code owners June 22, 2023 13:50

JGSweets reviewed Jun 22, 2023

View reviewed changes

dataprofiler/profilers/numerical_column_stats.py Outdated Show resolved Hide resolved

JGSweets reviewed Jun 22, 2023

View reviewed changes

dataprofiler/profilers/profiler_options.py Show resolved Hide resolved

taylorfturner assigned clee1152 Jun 22, 2023

taylorfturner added the New Feature A feature addition not currently in the library label Jun 22, 2023

taylorfturner suggested changes Jun 22, 2023

View reviewed changes

taylorfturner added the Work In Progress Solution is being developed label Jun 22, 2023

JGSweets reviewed Jun 23, 2023

View reviewed changes

dataprofiler/profilers/profiler_options.py Show resolved Hide resolved

clee1152 added 4 commits June 27, 2023 09:50

Implemented options for num_quantiles feature in NumericStatsMixin

4d2b6ad

Implemented options for num_quantiles.py.

47903d2

Added tests for num_quantiles.

4045c58

Fixed tests for num_quantiles option in column profiles.

ea603d2

clee1152 added 2 commits June 27, 2023 21:18

Added scipy requirements to requirements.txt

1c44351

Added tests for num_quantiles option.

de8104c

clee1152 requested a review from taylorfturner June 28, 2023 13:52

taylorfturner removed the Work In Progress Solution is being developed label Jun 28, 2023

taylorfturner enabled auto-merge (squash) June 28, 2023 15:41

taylorfturner mentioned this pull request Jun 28, 2023

Scipy Verision + Graph Issues #905

Closed

tyfarnan previously approved these changes Jun 28, 2023

View reviewed changes

taylorfturner suggested changes Jun 28, 2023

View reviewed changes

Update dataprofiler/profilers/profiler_options.py

26554ae

Co-authored-by: Taylor Turner <[email protected]>

auto-merge was automatically disabled June 28, 2023 17:25
Head branch was pushed to by a user without write access

clee1152 dismissed tyfarnan’s stale review via 26554ae June 28, 2023 17:25

clee1152 and others added 4 commits June 28, 2023 13:26

Update dataprofiler/tests/profilers/profiler_options/test_num_quantil…

382f11c

…es_option.py Co-authored-by: Taylor Turner <[email protected]>

Minor edits to tests

3a19c24

Empty commit

bc5f195

Precommits

48cbdf4

taylorfturner enabled auto-merge (squash) June 28, 2023 18:20

clee1152 requested a review from taylorfturner June 28, 2023 18:46

JGSweets reviewed Jul 5, 2023

View reviewed changes

taylorfturner changed the base branch from feature/options to feature/quantile_options July 6, 2023 12:55

taylorfturner added the Work In Progress Solution is being developed label Jul 6, 2023

clee1152 closed this Jul 10, 2023

auto-merge was automatically disabled July 10, 2023 16:52
Pull request was closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented options for `num_quantiles` feature in `NumericStatsMixin` #896

Implemented options for `num_quantiles` feature in `NumericStatsMixin` #896

clee1152 commented Jun 22, 2023

CLAassistant commented Jun 22, 2023 •

edited

Loading

taylorfturner left a comment

clee1152 commented Jun 23, 2023 •

edited

Loading

clee1152 commented Jun 27, 2023

taylorfturner commented Jun 27, 2023

clee1152 commented Jun 28, 2023

JGSweets Jul 5, 2023

taylorfturner Jul 5, 2023

JGSweets Jul 5, 2023

JGSweets Jul 5, 2023

clee1152 Jul 6, 2023

JGSweets Jul 6, 2023

clee1152 Jul 6, 2023

taylorfturner Jul 6, 2023

clee1152 commented Jul 10, 2023

		@@ -269,6 +269,40 @@ def _validate_helper(self, variable_path: str = "ModeOption") -> list[str]:
		return errors


		class NumQuantilesOption(BooleanOption):

Implemented options for num_quantiles feature in NumericStatsMixin #896

Implemented options for num_quantiles feature in NumericStatsMixin #896

Conversation

clee1152 commented Jun 22, 2023

CLAassistant commented Jun 22, 2023 • edited Loading

taylorfturner left a comment

Choose a reason for hiding this comment

clee1152 commented Jun 23, 2023 • edited Loading

clee1152 commented Jun 27, 2023

taylorfturner commented Jun 27, 2023

clee1152 commented Jun 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clee1152 commented Jul 10, 2023

Implemented options for `num_quantiles` feature in `NumericStatsMixin` #896

Implemented options for `num_quantiles` feature in `NumericStatsMixin` #896

CLAassistant commented Jun 22, 2023 •

edited

Loading

clee1152 commented Jun 23, 2023 •

edited

Loading