feat: enable feature store batch serve to Pandas DataFrame; fix: read instances uri for batch serve #983

morgandu · 2022-01-28T01:52:44Z

add batch_to_df
add unit tests
add integration tests
fix bugs

…pdate: setup.py; fix: read_instances bug for batch_serve_to_*

google/cloud/aiplatform/featurestore/featurestore.py

sasha-gitg · 2022-01-28T15:15:11Z

google/cloud/aiplatform/featurestore/featurestore.py

+            project=self.project, credentials=self.credentials
+        )
+
+        featurestore_name_components = self._parse_resource_name(self.resource_name)


It seems like this method should include a self.wait call because it's possible they could run methods like create and create_entity_type with sync=False and then call this method. That may be the case else where in these classes.

Right. For create_entity_type the sync is on the EntityType class, where the self.wait will be needed for _parse_resource_name for its entity_type_name_components.

Checked and updated all methods in featurestore module who may have this issue.

sasha-gitg · 2022-01-28T15:16:12Z

google/cloud/aiplatform/featurestore/featurestore.py

+        temp_bq_dataset_name = f"temp_{featurestore_id}_{uuid.uuid4()}".replace(
+            "-", "_"
+        )
+        temp_bq_dataset_id = f"{initializer.global_config.project}.{temp_bq_dataset_name}"[


Please add a TODO with with the project ID buganizer ID and description in locations where we reference the global config's project.

sasha-gitg · 2022-01-28T15:31:26Z

google/cloud/aiplatform/featurestore/featurestore.py

+            for message in reader.rows().pages:
+                frames.append(message.to_dataframe())
+
+        return None if not frames else pd.concat(frames)


Should this use ignore_index=True or is the SOT for the index in the table or possibly message.to_dataframe() assigns incrementing index values?

ignore_index=True is needed. Updated.

sasha-gitg · 2022-01-28T15:32:39Z

google/cloud/aiplatform/featurestore/featurestore.py

+            for message in reader.rows().pages:
+                frames.append(message.to_dataframe())
+
+        return None if not frames else pd.concat(frames)


This return type doesn't match the signature. I think we should return an empty pd.DataFrame if frames is empty.

…ex=True for pd.concat and return pd.DataFrame upon empty frames

diemtvu

lgtm

gcf-merge-on-green · 2022-02-04T12:20:55Z

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

feat: add batch_serve_to_df, add unit tests, add integration tests; u…

bcf2fc1

…pdate: setup.py; fix: read_instances bug for batch_serve_to_*

morgandu requested a review from sasha-gitg January 28, 2022 01:52

morgandu requested a review from a team as a code owner January 28, 2022 01:52

sasha-gitg requested changes Jan 28, 2022

View reviewed changes

morgandu added 2 commits January 31, 2022 17:04

Merge branch 'main' into mor--feature-store-batch-serve-to-df

b0b788d

fix: add self.wait() before self._parse_resource_name, add ignore_ind…

4b03f01

…ex=True for pd.concat and return pd.DataFrame upon empty frames

morgandu requested review from sasha-gitg and ivanmkc February 1, 2022 01:58

sasha-gitg approved these changes Feb 1, 2022

View reviewed changes

morgandu added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 1, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 1, 2022

morgandu added 3 commits February 1, 2022 13:38

fix: setup.py versioning conflict

e7467e9

Merge branch 'main' into mor--feature-store-batch-serve-to-df

7b27082

Merge branch 'main' into mor--feature-store-batch-serve-to-df

c7c9c07

diemtvu approved these changes Feb 2, 2022

View reviewed changes

morgandu added 2 commits February 3, 2022 15:34

Merge branch 'main' into mor--feature-store-batch-serve-to-df

472de1e

fix: optional_sync

290b07d

morgandu added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Feb 4, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 4, 2022

morgandu force-pushed the mor--feature-store-batch-serve-to-df branch from 2cad0d4 to ca0d7bf Compare February 4, 2022 06:19

morgandu added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 4, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 4, 2022

gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Feb 4, 2022

morgandu added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 4, 2022

yoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Feb 4, 2022

morgandu added the automerge Merge the pull request once unit tests and other checks pass. label Feb 4, 2022

morgandu changed the title ~~feat: enable batch_serve_to_df; fix: read instances bugs for batch serve~~ feat: enable feature store batch serve to Pandas DataFrame; fix: read instances for batch serve Feb 4, 2022

morgandu changed the title ~~feat: enable feature store batch serve to Pandas DataFrame; fix: read instances for batch serve~~ feat: enable feature store batch serve to Pandas DataFrame; fix: read instances uri for batch serve Feb 4, 2022

morgandu force-pushed the mor--feature-store-batch-serve-to-df branch from ca0d7bf to 290b07d Compare February 4, 2022 18:13

morgandu added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed automerge Merge the pull request once unit tests and other checks pass. labels Feb 4, 2022

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 4, 2022

morgandu merged commit e0fec36 into googleapis:main Feb 4, 2022

release-please bot mentioned this pull request Feb 4, 2022

chore(main): release 1.10.0 #951

Merged

morgandu self-assigned this Feb 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable feature store batch serve to Pandas DataFrame; fix: read instances uri for batch serve #983

feat: enable feature store batch serve to Pandas DataFrame; fix: read instances uri for batch serve #983

morgandu commented Jan 28, 2022

sasha-gitg Jan 28, 2022

morgandu Jan 31, 2022 •

edited

Loading

sasha-gitg Jan 28, 2022

morgandu Jan 31, 2022

sasha-gitg Jan 28, 2022

morgandu Feb 1, 2022 •

edited

Loading

sasha-gitg Jan 28, 2022

morgandu Jan 31, 2022

diemtvu left a comment

gcf-merge-on-green bot commented Feb 4, 2022

feat: enable feature store batch serve to Pandas DataFrame; fix: read instances uri for batch serve #983

feat: enable feature store batch serve to Pandas DataFrame; fix: read instances uri for batch serve #983

Conversation

morgandu commented Jan 28, 2022

sasha-gitg Jan 28, 2022

Choose a reason for hiding this comment

morgandu Jan 31, 2022 • edited Loading

Choose a reason for hiding this comment

sasha-gitg Jan 28, 2022

Choose a reason for hiding this comment

morgandu Jan 31, 2022

Choose a reason for hiding this comment

sasha-gitg Jan 28, 2022

Choose a reason for hiding this comment

morgandu Feb 1, 2022 • edited Loading

Choose a reason for hiding this comment

sasha-gitg Jan 28, 2022

Choose a reason for hiding this comment

morgandu Jan 31, 2022

Choose a reason for hiding this comment

diemtvu left a comment

Choose a reason for hiding this comment

gcf-merge-on-green bot commented Feb 4, 2022

morgandu Jan 31, 2022 •

edited

Loading

morgandu Feb 1, 2022 •

edited

Loading