fix: Study.list() method (#1594)

jaycee-li · web-flow · commit 47eb0aef224f · 2022-08-18T12:21:28.000-07:00
* fix: high level list method * Revert "fix: high level list method" This reverts commit 17c9bdd. * Revert "Revert "fix: high level list method"" This reverts commit b4308fd. * fix: unit tests for list() * fix: blacken
diff --git a/google/cloud/aiplatform/base.py b/google/cloud/aiplatform/base.py
@@ -1,6 +1,6 @@
 # -*- coding: utf-8 -*-
 
-# Copyright 2020 Google LLC
+# Copyright 2022 Google LLC
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -1077,9 +1077,11 @@ def _list(
             or initializer.global_config.common_location_path(
                 project=project, location=location
             ),
-            "filter": filter,
         }
 
+        if filter:
+            list_request["filter"] = filter
+
         if order_by:
             list_request["order_by"] = order_by
 
diff --git a/tests/unit/aiplatform/test_datasets.py b/tests/unit/aiplatform/test_datasets.py
@@ -1421,9 +1421,7 @@ def test_list_dataset_no_order_or_filter(self, list_datasets_mock):
 
         ds_list = aiplatform.TabularDataset.list()
 
-        list_datasets_mock.assert_called_once_with(
-            request={"parent": _TEST_PARENT, "filter": None}
-        )
+        list_datasets_mock.assert_called_once_with(request={"parent": _TEST_PARENT})
 
         # Ensure returned list is smaller since it filtered out non-tabular datasets
         assert len(ds_list) < len(_TEST_DATASET_LIST)
diff --git a/tests/unit/aiplatform/test_featurestores.py b/tests/unit/aiplatform/test_featurestores.py
@@ -984,7 +984,7 @@ def test_list_featurestores(self, list_featurestores_mock):
         my_featurestore_list = aiplatform.Featurestore.list()
 
         list_featurestores_mock.assert_called_once_with(
-            request={"parent": _TEST_PARENT, "filter": None}
+            request={"parent": _TEST_PARENT}
         )
         assert len(my_featurestore_list) == len(_TEST_FEATURESTORE_LIST)
         for my_featurestore in my_featurestore_list:
@@ -1028,7 +1028,7 @@ def test_list_entity_types(self, list_entity_types_mock):
         my_entity_type_list = my_featurestore.list_entity_types()
 
         list_entity_types_mock.assert_called_once_with(
-            request={"parent": _TEST_FEATURESTORE_NAME, "filter": None}
+            request={"parent": _TEST_FEATURESTORE_NAME}
         )
         assert len(my_entity_type_list) == len(_TEST_ENTITY_TYPE_LIST)
         for my_entity_type in my_entity_type_list:
@@ -1770,7 +1770,7 @@ def test_list_entity_types(self, featurestore_name, list_entity_types_mock):
         )
 
         list_entity_types_mock.assert_called_once_with(
-            request={"parent": _TEST_FEATURESTORE_NAME, "filter": None}
+            request={"parent": _TEST_FEATURESTORE_NAME}
         )
         assert len(my_entity_type_list) == len(_TEST_ENTITY_TYPE_LIST)
         for my_entity_type in my_entity_type_list:
@@ -1784,7 +1784,7 @@ def test_list_features(self, list_features_mock):
         my_feature_list = my_entity_type.list_features()
 
         list_features_mock.assert_called_once_with(
-            request={"parent": _TEST_ENTITY_TYPE_NAME, "filter": None}
+            request={"parent": _TEST_ENTITY_TYPE_NAME}
         )
         assert len(my_feature_list) == len(_TEST_FEATURE_LIST)
         for my_feature in my_feature_list:
@@ -2851,7 +2851,7 @@ def test_list_features(self, entity_type_name, featurestore_id, list_features_mo
         )
 
         list_features_mock.assert_called_once_with(
-            request={"parent": _TEST_ENTITY_TYPE_NAME, "filter": None}
+            request={"parent": _TEST_ENTITY_TYPE_NAME}
         )
         assert len(my_feature_list) == len(_TEST_FEATURE_LIST)
         for my_feature in my_feature_list:
diff --git a/tests/unit/aiplatform/test_matching_engine_index.py b/tests/unit/aiplatform/test_matching_engine_index.py
@@ -246,9 +246,7 @@ def test_list_indexes(self, list_indexes_mock):
 
         my_indexes_list = aiplatform.MatchingEngineIndex.list()
 
-        list_indexes_mock.assert_called_once_with(
-            request={"parent": _TEST_PARENT, "filter": None}
-        )
+        list_indexes_mock.assert_called_once_with(request={"parent": _TEST_PARENT})
         assert len(my_indexes_list) == len(_TEST_INDEX_LIST)
         for my_index in my_indexes_list:
             assert type(my_index) == aiplatform.MatchingEngineIndex
diff --git a/tests/unit/aiplatform/test_matching_engine_index_endpoint.py b/tests/unit/aiplatform/test_matching_engine_index_endpoint.py
@@ -440,7 +440,7 @@ def test_list_index_endpoints(self, list_index_endpoints_mock):
         my_index_endpoints_list = aiplatform.MatchingEngineIndexEndpoint.list()
 
         list_index_endpoints_mock.assert_called_once_with(
-            request={"parent": _TEST_PARENT, "filter": None}
+            request={"parent": _TEST_PARENT}
         )
         assert len(my_index_endpoints_list) == len(_TEST_INDEX_ENDPOINT_LIST)
         for my_index_endpoint in my_index_endpoints_list:
diff --git a/tests/unit/aiplatform/test_models.py b/tests/unit/aiplatform/test_models.py
@@ -2382,7 +2382,7 @@ def test_get_model_evaluation_without_id(
         test_model.get_model_evaluation()
 
         list_model_evaluations_mock.assert_called_once_with(
-            request={"parent": _TEST_MODEL_RESOURCE_NAME, "filter": None}
+            request={"parent": _TEST_MODEL_RESOURCE_NAME}
         )
 
     def test_list_model_evaluations(
@@ -2397,7 +2397,7 @@ def test_list_model_evaluations(
         eval_list = test_model.list_model_evaluations()
 
         list_model_evaluations_mock.assert_called_once_with(
-            request={"parent": _TEST_MODEL_RESOURCE_NAME, "filter": None}
+            request={"parent": _TEST_MODEL_RESOURCE_NAME}
         )
 
         assert len(eval_list) == len(_TEST_MODEL_EVAL_LIST)
diff --git a/tests/unit/aiplatform/test_pipeline_jobs.py b/tests/unit/aiplatform/test_pipeline_jobs.py
@@ -1289,7 +1289,7 @@ def test_list_pipeline_job(
         job.list()
 
         mock_pipeline_service_list.assert_called_once_with(
-            request={"parent": _TEST_PARENT, "filter": None}
+            request={"parent": _TEST_PARENT}
         )
 
     @pytest.mark.usefixtures(
diff --git a/tests/unit/aiplatform/test_tensorboard.py b/tests/unit/aiplatform/test_tensorboard.py
@@ -735,7 +735,7 @@ def test_list_tensorboard_experiments(self, list_tensorboard_experiment_mock):
         tensorboard.TensorboardExperiment.list(tensorboard_name=_TEST_NAME)
 
         list_tensorboard_experiment_mock.assert_called_once_with(
-            request={"parent": _TEST_NAME, "filter": None}
+            request={"parent": _TEST_NAME}
         )
 
 
@@ -785,7 +785,7 @@ def test_init_tensorboard_run_with_id_only_with_project_and_location(
             name=_TEST_TENSORBOARD_RUN_NAME, retry=base._DEFAULT_RETRY
         )
         list_tensorboard_time_series_mock.assert_called_once_with(
-            request={"parent": _TEST_TENSORBOARD_RUN_NAME, "filter": None}
+            request={"parent": _TEST_TENSORBOARD_RUN_NAME}
         )
 
     @pytest.mark.usefixtures("list_tensorboard_time_series_mock")
@@ -900,11 +900,11 @@ def test_list_tensorboard_runs(
         )
 
         list_tensorboard_run_mock.assert_called_once_with(
-            request={"parent": _TEST_TENSORBOARD_EXPERIMENT_NAME, "filter": None}
+            request={"parent": _TEST_TENSORBOARD_EXPERIMENT_NAME}
         )
 
         list_tensorboard_time_series_mock.assert_called_once_with(
-            request={"parent": _TEST_TENSORBOARD_RUN_NAME, "filter": None}
+            request={"parent": _TEST_TENSORBOARD_RUN_NAME}
         )
 
     @pytest.mark.usefixtures(
@@ -1064,5 +1064,5 @@ def test_list_tensorboard_time_series(self, list_tensorboard_time_series_mock):
         )
 
         list_tensorboard_time_series_mock.assert_called_once_with(
-            request={"parent": _TEST_TENSORBOARD_RUN_NAME, "filter": None}
+            request={"parent": _TEST_TENSORBOARD_RUN_NAME}
         )

Original file line number	Diff line number	Diff line change
`@@ -984,7 +984,7 @@ def test_list_featurestores(self, list_featurestores_mock):`
`984`	`984`	`my_featurestore_list = aiplatform.Featurestore.list()`
`985`	`985`
`986`	`986`	`list_featurestores_mock.assert_called_once_with(`
`987`		`- request={"parent": _TEST_PARENT, "filter": None}`
	`987`	`+ request={"parent": _TEST_PARENT}`
`988`	`988`	`)`
`989`	`989`	`assert len(my_featurestore_list) == len(_TEST_FEATURESTORE_LIST)`
`990`	`990`	`for my_featurestore in my_featurestore_list:`
`@@ -1028,7 +1028,7 @@ def test_list_entity_types(self, list_entity_types_mock):`
`1028`	`1028`	`my_entity_type_list = my_featurestore.list_entity_types()`
`1029`	`1029`
`1030`	`1030`	`list_entity_types_mock.assert_called_once_with(`
`1031`		`- request={"parent": _TEST_FEATURESTORE_NAME, "filter": None}`
	`1031`	`+ request={"parent": _TEST_FEATURESTORE_NAME}`
`1032`	`1032`	`)`
`1033`	`1033`	`assert len(my_entity_type_list) == len(_TEST_ENTITY_TYPE_LIST)`
`1034`	`1034`	`for my_entity_type in my_entity_type_list:`
`@@ -1770,7 +1770,7 @@ def test_list_entity_types(self, featurestore_name, list_entity_types_mock):`
`1770`	`1770`	`)`
`1771`	`1771`
`1772`	`1772`	`list_entity_types_mock.assert_called_once_with(`
`1773`		`- request={"parent": _TEST_FEATURESTORE_NAME, "filter": None}`
	`1773`	`+ request={"parent": _TEST_FEATURESTORE_NAME}`
`1774`	`1774`	`)`
`1775`	`1775`	`assert len(my_entity_type_list) == len(_TEST_ENTITY_TYPE_LIST)`
`1776`	`1776`	`for my_entity_type in my_entity_type_list:`
`@@ -1784,7 +1784,7 @@ def test_list_features(self, list_features_mock):`
`1784`	`1784`	`my_feature_list = my_entity_type.list_features()`
`1785`	`1785`
`1786`	`1786`	`list_features_mock.assert_called_once_with(`
`1787`		`- request={"parent": _TEST_ENTITY_TYPE_NAME, "filter": None}`
	`1787`	`+ request={"parent": _TEST_ENTITY_TYPE_NAME}`
`1788`	`1788`	`)`
`1789`	`1789`	`assert len(my_feature_list) == len(_TEST_FEATURE_LIST)`
`1790`	`1790`	`for my_feature in my_feature_list:`
`@@ -2851,7 +2851,7 @@ def test_list_features(self, entity_type_name, featurestore_id, list_features_mo`
`2851`	`2851`	`)`
`2852`	`2852`
`2853`	`2853`	`list_features_mock.assert_called_once_with(`
`2854`		`- request={"parent": _TEST_ENTITY_TYPE_NAME, "filter": None}`
	`2854`	`+ request={"parent": _TEST_ENTITY_TYPE_NAME}`
`2855`	`2855`	`)`
`2856`	`2856`	`assert len(my_feature_list) == len(_TEST_FEATURE_LIST)`
`2857`	`2857`	`for my_feature in my_feature_list:`
Original file line number	Diff line number	Diff line change
`@@ -440,7 +440,7 @@ def test_list_index_endpoints(self, list_index_endpoints_mock):`
`440`	`440`	`my_index_endpoints_list = aiplatform.MatchingEngineIndexEndpoint.list()`
`441`	`441`
`442`	`442`	`list_index_endpoints_mock.assert_called_once_with(`
`443`		`- request={"parent": _TEST_PARENT, "filter": None}`
	`443`	`+ request={"parent": _TEST_PARENT}`
`444`	`444`	`)`
`445`	`445`	`assert len(my_index_endpoints_list) == len(_TEST_INDEX_ENDPOINT_LIST)`
`446`	`446`	`for my_index_endpoint in my_index_endpoints_list:`
Original file line number	Diff line number	Diff line change
`@@ -2382,7 +2382,7 @@ def test_get_model_evaluation_without_id(`
`2382`	`2382`	`test_model.get_model_evaluation()`
`2383`	`2383`
`2384`	`2384`	`list_model_evaluations_mock.assert_called_once_with(`
`2385`		`- request={"parent": _TEST_MODEL_RESOURCE_NAME, "filter": None}`
	`2385`	`+ request={"parent": _TEST_MODEL_RESOURCE_NAME}`
`2386`	`2386`	`)`
`2387`	`2387`
`2388`	`2388`	`def test_list_model_evaluations(`
`@@ -2397,7 +2397,7 @@ def test_list_model_evaluations(`
`2397`	`2397`	`eval_list = test_model.list_model_evaluations()`
`2398`	`2398`
`2399`	`2399`	`list_model_evaluations_mock.assert_called_once_with(`
`2400`		`- request={"parent": _TEST_MODEL_RESOURCE_NAME, "filter": None}`
	`2400`	`+ request={"parent": _TEST_MODEL_RESOURCE_NAME}`
`2401`	`2401`	`)`
`2402`	`2402`
`2403`	`2403`	`assert len(eval_list) == len(_TEST_MODEL_EVAL_LIST)`
Original file line number	Diff line number	Diff line change
`@@ -1289,7 +1289,7 @@ def test_list_pipeline_job(`
`1289`	`1289`	`job.list()`
`1290`	`1290`
`1291`	`1291`	`mock_pipeline_service_list.assert_called_once_with(`
`1292`		`- request={"parent": _TEST_PARENT, "filter": None}`
	`1292`	`+ request={"parent": _TEST_PARENT}`
`1293`	`1293`	`)`
`1294`	`1294`
`1295`	`1295`	`@pytest.mark.usefixtures(`
Original file line number	Diff line number	Diff line change
`@@ -735,7 +735,7 @@ def test_list_tensorboard_experiments(self, list_tensorboard_experiment_mock):`
`735`	`735`	`tensorboard.TensorboardExperiment.list(tensorboard_name=_TEST_NAME)`
`736`	`736`
`737`	`737`	`list_tensorboard_experiment_mock.assert_called_once_with(`
`738`		`- request={"parent": _TEST_NAME, "filter": None}`
	`738`	`+ request={"parent": _TEST_NAME}`
`739`	`739`	`)`
`740`	`740`
`741`	`741`
`@@ -785,7 +785,7 @@ def test_init_tensorboard_run_with_id_only_with_project_and_location(`
`785`	`785`	`name=_TEST_TENSORBOARD_RUN_NAME, retry=base._DEFAULT_RETRY`
`786`	`786`	`)`
`787`	`787`	`list_tensorboard_time_series_mock.assert_called_once_with(`
`788`		`- request={"parent": _TEST_TENSORBOARD_RUN_NAME, "filter": None}`
	`788`	`+ request={"parent": _TEST_TENSORBOARD_RUN_NAME}`
`789`	`789`	`)`
`790`	`790`
`791`	`791`	`@pytest.mark.usefixtures("list_tensorboard_time_series_mock")`
`@@ -900,11 +900,11 @@ def test_list_tensorboard_runs(`
`900`	`900`	`)`
`901`	`901`
`902`	`902`	`list_tensorboard_run_mock.assert_called_once_with(`
`903`		`- request={"parent": _TEST_TENSORBOARD_EXPERIMENT_NAME, "filter": None}`
	`903`	`+ request={"parent": _TEST_TENSORBOARD_EXPERIMENT_NAME}`
`904`	`904`	`)`
`905`	`905`
`906`	`906`	`list_tensorboard_time_series_mock.assert_called_once_with(`
`907`		`- request={"parent": _TEST_TENSORBOARD_RUN_NAME, "filter": None}`
	`907`	`+ request={"parent": _TEST_TENSORBOARD_RUN_NAME}`
`908`	`908`	`)`
`909`	`909`
`910`	`910`	`@pytest.mark.usefixtures(`
`@@ -1064,5 +1064,5 @@ def test_list_tensorboard_time_series(self, list_tensorboard_time_series_mock):`
`1064`	`1064`	`)`
`1065`	`1065`
`1066`	`1066`	`list_tensorboard_time_series_mock.assert_called_once_with(`
`1067`		`- request={"parent": _TEST_TENSORBOARD_RUN_NAME, "filter": None}`
	`1067`	`+ request={"parent": _TEST_TENSORBOARD_RUN_NAME}`
`1068`	`1068`	`)`