Updated null stats tests to include data initialized in setUpClass #898
Conversation
jacob-buehler
requested review from
JGSweets,
ksneab7,
taylorfturner,
micdavis and
tyfarnan
as code owners
|
|
| from . import utils as test_utils | ||
| from dataprofiler.tests.profilers import utils as test_utils |
There was a problem hiding this comment.
why did this need to change?
There was a problem hiding this comment.
the original line didn't import utils properly on my local, but the new line imports it consistently
There was a problem hiding this comment.
That might be bc the test was run from the dir as opposed to base of repo. I think here we are trying to separate from test vs in lib by using relative. but open to convo if we need a change here.
| # file_path = os.path.join(test_root_path, "data", "csv/empty_rows.txt") | ||
| # data = pd.read_csv(file_path) |
| # I commented out these lines of code, because they are a second | ||
| # test of the functions tested in the last four lines of code. | ||
| # Since we intend to use only the setUpClass data, there is no | ||
| # reason to keep these in, or test those functions a second time. | ||
|
|
||
| # file_path = os.path.join(test_root_path, "data", "csv/iris-with-null-rows.csv") | ||
| # data = pd.read_csv(file_path) | ||
| # data = self.data | ||
|
|
||
| # profile = dp.StructuredProfiler(data, options=profiler_options) | ||
| # self.assertEqual(13, profile.row_has_null_count) | ||
| # self.assertEqual(13 / 24, profile._get_row_has_null_ratio()) | ||
| # self.assertEqual(3, profile.row_is_null_count) | ||
| # self.assertEqual(3 / 24, profile._get_row_is_null_ratio()) |
There was a problem hiding this comment.
i'd recommend either deleting or keeping -- not just keeping commented out code in the file
| @@ -3646,35 +3665,39 @@ def test_null_in_file(self): | |||
| "row_statistics.is_enabled": True, | |||
| } | |||
| ) | |||
| data = dp.Data(filename_null_in_file) | |||
| # data = dp.Data(filename_null_in_file) | |||
There was a problem hiding this comment.
do we need to keep commented out?
| names_idx = report["global_stats"]["profile_schema"]["names"][0] | ||
| numbers_idx = report["global_stats"]["profile_schema"]["numbers"][0] |
There was a problem hiding this comment.
changing becuase of new dataset
| # data = [ | ||
| # ["test1", 1.0], | ||
| # ["test2", 2.0], | ||
| # ["test3", 3.0], | ||
| # [None, None], | ||
| # ["test5", 5.0], | ||
| # ["test6", 6.0], | ||
| # [None, None], | ||
| # ["test7", 7.0], | ||
| # ] | ||
| # data = pd.DataFrame(data, columns=["NAME", "VALUE"]) |
There was a problem hiding this comment.
same as above on commented out code
| # data = pd.DataFrame( | ||
| # { | ||
| # "full": [1, 2, 3, 4, 5, 6, 7, 8, 9], | ||
| # "sparse": [1, None, 3, None, 5, None, 7, None, 9], | ||
| # } | ||
| # ) |
| # data2 = pd.DataFrame( | ||
| # { | ||
| # "sparse": [1, None, 3, None, 5, None, 7, None], | ||
| # "sparser": [1, None, None, None, None, None, None, 8], | ||
| # } | ||
| # ) |
| # self.assertSetEqual({}, profile._profile[0].null_types_index) | ||
| # self.assertSetEqual({}, profile._profile[1].null_types_index) | ||
| self.assertEqual({}, profile._profile[0].null_types_index) | ||
| self.assertEqual({}, profile._profile[1].null_types_index) |
There was a problem hiding this comment.
why is this changing from a set to just assertEqual?
There was a problem hiding this comment.
to solve an issue: 'dict' has no attribute 'difference'
Head branch was pushed to by a user without write access
| file_path = os.path.join(test_root_path, "data", "csv/empty_rows.txt") | ||
| data = pd.read_csv(file_path) | ||
| data = self.data | ||
|
|
There was a problem hiding this comment.
I wonder if there was intent to profile an empty csv. I'm concerned about changing the data.
There was a problem hiding this comment.
I think we may need to take a step back here and identify the need of this PR.
Identify which tests in here are slow and work towards what we need to do to speed those up as oppose to a changing everything. I'm concerned by unifying to a single data source we are losing data variability which has intention within the structure of the data to test different cases of profiling.
|
Related to issue #866 |
Updated null stats tests to include data initialized in setUpClass function