StructuredOptions: `hhl_row_hashing` #841

micdavis · 2023-05-24T15:39:18Z

added hhl_row_hashing option
Default is original row hashing method: hhl_row_hashing=False

micdavis · 2023-05-24T15:40:40Z

interested to know if hll_row_hashing_enabled makes more sense, or something different that what I have

JGSweets · 2023-05-24T16:11:44Z

dataprofiler/profilers/profile_builder.py

+        if not self.options.hll_row_hashing:
+            try:
+                self.hashed_row_dict.update(
+                    dict.fromkeys(pd.util.hash_pandas_object(data, index=False), True)
+                )
+            except TypeError:
+                self.hashed_row_dict.update(
+                    dict.fromkeys(
+                        pd.util.hash_pandas_object(data.astype(str), index=False), True
+                    )


Let's not change the profile at all when pr the option set. Any logical usage in the profile can be after.

ready for re-review

JGSweets · 2023-05-24T16:14:18Z

dataprofiler/profilers/profiler_options.py

@@ -1228,15 +1229,17 @@ def __init__(
        # Non-Option variables
        self.null_values = null_values
        self.column_null_values = column_null_values
+        self.hll_row_hashing = hll_row_hashing


this needs to be under row_statistics since that's the hierarchy in the code. We will have to make a class for row statistics instead of using booleanoptions which will be similar to correlations like on https://github.com/capitalone/DataProfiler/pull/841/files#diff-00da4b3383cf04d1ea70e7d179d78433f0bfde3ac6909c791eb92363c5a34f3bR1225

ready for re-review

micdavis · 2023-05-30T14:54:07Z

dataprofiler/profilers/profiler_options.py

@@ -982,6 +982,64 @@ def _validate_helper(self, variable_path: str = "CorrelationOptions") -> list[st
        return errors


+class HyperLogLogOptions(BooleanOption):


Created a nested HLL options class because seed is only pertinent when HLL is enabled.

Let's rename to UniqueCountOptions. Here we should have a str choice to use full or hll_row_hashing in addition we can have the hll_options.

ready for re-review

micdavis · 2023-05-30T14:56:03Z

dataprofiler/tests/profilers/profiler_options/test_row_statistics_options.py

@@ -0,0 +1,71 @@
+from dataprofiler.profilers.profiler_options import RowStatisticsOptions


Unsure about these tests, not sure how to handle nested option testing.

micdavis · 2023-05-30T17:08:12Z

dataprofiler/profilers/profiler_options.py

+        :vartype seed: int
+        """
+        BooleanOption.__init__(self, is_enabled=is_enabled)
+        self.seed = seed


should I also include "p" which represents the 2^p registers in the HLL table

Yes per conversation with the team we should implement and option for this

taylorfturner

blocking for 0.9.0 release

…is/DataProfiler into hashed_row_object_options

JGSweets · 2023-06-05T17:52:51Z

dataprofiler/profilers/profiler_options.py

+        :vartype is_enabled: bool
+        """
+        BooleanOption.__init__(self, is_enabled=is_enabled)
+        self.hll_row_hashing = HyperLogLogOptions(is_enabled=False)


instead of hll_row_hashing, this is an option wrt the hashes for unique_count. Hence, we should do more like:
self.unique_count so we can enable or disable that. Then, under unique_count we can have the choice between using hll_row_hashing or full_hashing w/ the ability to set the parameters for hll.

changed, lmk your thoughts

taylorfturner · 2023-06-05T19:11:20Z

dataprofiler/profilers/profiler_options.py

@@ -982,6 +982,74 @@ def _validate_helper(self, variable_path: str = "CorrelationOptions") -> list[st
        return errors


+class HyperLogLogOptions(BooleanOption):


agree -- let's make this a UniqueCountOptions with more granular toggles of full or hll_row_hashing options beneath that.

JGSweets · 2023-06-06T15:06:30Z

dataprofiler/tests/profilers/profiler_options/test_hyper_log_log_options.py

+        optpth = self.get_options_path()
+
+        # Default configuration
+        option = self.get_options(seed=0, register_count=10)


In order to test default, I suggest not overriding. i.e. just do option = self.get_options()

…is/DataProfiler into hashed_row_object_options

micdavis · 2023-06-06T15:49:16Z

dataprofiler/profilers/profiler_options.py

+        return errors
+
+
+class UniqueCountOptions(BooleanOption):


curious if RowStatisticsOptions, UniqueCountOptions, or HyperLogLogOptions should inherit BooleanOption or BaseInspectorOptions

I'm wondering if it should be a BaseInspectorOption (e.g. TextProfilerOptions in UnstructuredOptions)

micdavis · 2023-06-06T15:49:37Z

dataprofiler/profilers/profiler_options.py

+        :vartype is_enabled: bool
+        """
+        BooleanOption.__init__(self, is_enabled=is_enabled)
+        self.full_hashing = BooleanOption(is_enabled=full_hashing)


should full_hashing be a BooleanOption?

I think this would be a boolean since there are no properties for full_hashing ... its just True / False binary

This is a good idea, but the issue is what happens if someone specifies true for both? Instead, we might want a string input that selects one or the other. good call below in the validate. Or we need a way to toggle. Open to thoughts.

…olean options now

added 'hhl_row_hashing' option

4df0055

micdavis requested review from JGSweets, ksneab7, taylorfturner and tyfarnan as code owners May 24, 2023 15:39

Merge branch 'main' into hashed_row_object_options

72ff644

taylorfturner assigned micdavis May 24, 2023

taylorfturner enabled auto-merge (squash) May 24, 2023 15:41

taylorfturner added Refactor Code that is being modified to improve the library Mend: license policy violation License policy violation detected by WhiteSource and removed Mend: license policy violation License policy violation detected by WhiteSource labels May 24, 2023

JGSweets reviewed May 24, 2023

View reviewed changes

micdavis mentioned this pull request May 24, 2023

StructuredProfiler: HLLRowHashing #842

Merged

created RowStatisticsOptions and HyperLogLogOptions and added tests

277b132

micdavis commented May 30, 2023

View reviewed changes

micdavis and others added 4 commits May 30, 2023 10:56

removed hll_row_hashing from StructuredOptions' prop_check

a3d15e7

fixed StructuredOptions test

d6d29ea

removed one un-needed line

fbf8cb5

Merge branch 'main' into hashed_row_object_options

1472934

micdavis commented May 30, 2023

View reviewed changes

Merge branch 'main' into hashed_row_object_options

983e4c5

taylorfturner suggested changes May 31, 2023

View reviewed changes

micdavis and others added 3 commits May 31, 2023 12:53

added register_count as an additional option

fbd0095

Merge branch 'hashed_row_object_options' of https://github.com/micdav…

5bfb033

…is/DataProfiler into hashed_row_object_options

Merge branch 'main' into hashed_row_object_options

3f6a928

JGSweets reviewed Jun 5, 2023

View reviewed changes

taylorfturner suggested changes Jun 5, 2023

View reviewed changes

micdavis added the Work In Progress Solution is being developed label Jun 6, 2023

Merge branch 'main' into hashed_row_object_options

fdd930a

JGSweets reviewed Jun 6, 2023

View reviewed changes

micdavis added 2 commits June 6, 2023 11:40

updated RowStatisticOptions structure

b2d4c87

Merge branch 'hashed_row_object_options' of https://github.com/micdav…

1aaf42e

…is/DataProfiler into hashed_row_object_options

micdavis removed the Work In Progress Solution is being developed label Jun 6, 2023

micdavis commented Jun 6, 2023

View reviewed changes

micdavis added 3 commits June 6, 2023 11:52

small change to test

e035456

updated options structure

211dfd6

updated test, failed because class inherits baseoptions instead of bo…

a051eab

…olean options now

JGSweets approved these changes Jun 6, 2023

View reviewed changes

micdavis mentioned this pull request Jun 6, 2023

RowStatisticsOptions: Add null row count #859

Closed

taylorfturner approved these changes Jun 6, 2023

View reviewed changes

taylorfturner merged commit 0f2c916 into capitalone:main Jun 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

StructuredOptions: `hhl_row_hashing` #841

StructuredOptions: `hhl_row_hashing` #841

micdavis commented May 24, 2023

micdavis commented May 24, 2023

JGSweets May 24, 2023

micdavis May 30, 2023

JGSweets May 24, 2023

micdavis May 30, 2023

micdavis May 30, 2023

JGSweets Jun 5, 2023

micdavis Jun 6, 2023

micdavis May 30, 2023

micdavis May 30, 2023

ksneab7 May 31, 2023

taylorfturner left a comment

JGSweets Jun 5, 2023

micdavis Jun 6, 2023

taylorfturner Jun 5, 2023

JGSweets Jun 6, 2023

taylorfturner Jun 6, 2023

micdavis Jun 6, 2023

micdavis Jun 6, 2023

taylorfturner Jun 6, 2023

taylorfturner Jun 6, 2023 •

edited

Loading

micdavis Jun 6, 2023

taylorfturner Jun 6, 2023

JGSweets Jun 6, 2023

		@@ -982,6 +982,64 @@ def _validate_helper(self, variable_path: str = "CorrelationOptions") -> list[st
		return errors


		class HyperLogLogOptions(BooleanOption):

		@@ -0,0 +1,71 @@
		from dataprofiler.profilers.profiler_options import RowStatisticsOptions

		@@ -982,6 +982,74 @@ def _validate_helper(self, variable_path: str = "CorrelationOptions") -> list[st
		return errors


		class HyperLogLogOptions(BooleanOption):

StructuredOptions: hhl_row_hashing #841

StructuredOptions: hhl_row_hashing #841

Conversation

micdavis commented May 24, 2023

micdavis commented May 24, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taylorfturner left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taylorfturner Jun 6, 2023 • edited Loading

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

StructuredOptions: `hhl_row_hashing` #841

StructuredOptions: `hhl_row_hashing` #841

taylorfturner Jun 6, 2023 •

edited

Loading