Implement initial Dataset class by vinnysenthil · Pull Request #31 · googleapis/python-aiplatform

vinnysenthil · Oct 20, 2020

Summary of changes:

Implement Dataset class with following methods
- __init__
- _create
- _import
- create
- import_data
- export_data
Ten unit tests covering 98% of Dataset class
Add two helper functions to utils.py — validate_id and validate_string_list
Add an error when project is not initialized or set in credentials

* Add global_config unset project error * Add two validation functions to utils * Implement initial Dataset class * Lint utils.py

sasha-gitg

Looks great! I really like how you used pytest fixtures for patching.

dizcology · Oct 20, 2020

+        if not source:
+            return dataset_obj
+
+        return dataset_obj.import_data(


This would return an LRO, while in the "no data import" case, we wait until the create_dataset LRO completes. We need to document clearly (e.g. in the docstring) this inconsistent behavior.

Yeah these multiple LROs under-the-hood can be misleading to the user. I'm adding this discussion to b/171275584

dizcology · Oct 20, 2020

+    @pytest.fixture
+    def get_dataset_mock(self):
+        with patch.object(DatasetServiceClient, "get_dataset") as get_dataset_mock:
+            get_dataset_mock.return_value = GapicDataset(


this can become difficult to maintain to make sure the mocks return correct responses. we should look into replays.

Captured in b/171334022

dizcology · Oct 20, 2020

+        reload(aip)
+
+    @pytest.fixture
+    def get_dataset_mock(self):


perhaps move the mock fixtures into conftest.py so they could be more easily reused.

Captured in b/171333554

* Dataset class (#1) * Add global_config unset project error * Add two validation functions to utils * Implement initial Dataset class * Lint utils.py * Address reviewer comments + remove aip alias * Change re.Match to typing.Match * Lint with Py 3.8 * Address flake8 errors, remove unused vars

Dataset class (#1)

eb601cd

* Add global_config unset project error * Add two validation functions to utils * Implement initial Dataset class * Lint utils.py

vinnysenthil requested review from dizcology and sasha-gitg October 20, 2020 06:52

google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 20, 2020

sasha-gitg approved these changes Oct 20, 2020

View reviewed changes