fix: OnDemandFeatureView type inference for array types by alexmirrington · Pull Request #4310 · feast-dev/feast

alexmirrington · 2024-06-24T09:41:15Z

What and Why

OnDemandFeatureView.feature_transformation.infer_features should be able to infer features from python types for all supported feast data types, for all transformation backends. This PR passes values to python_type_to_feast_value_type so that list types can be inferred correctly.

Tests

Unit tests passing in CI for all Python versions on my fork.
Tested against a local project by patching PandasTransformation.infer_features, e.g.

from feast.transformation.pandas_transformation import PandasTransformation

def __patched_infer_features(
    self, random_input: dict[str, list[Any]]
) -> list[Field]:
    df = pd.DataFrame.from_dict(random_input)
    output_df: pd.DataFrame = self.transform(df)
    return [
        Field(
            name=f,
            dtype=from_value_type(
                python_type_to_feast_value_type(
                    f, value=output_df[f].tolist()[0], type_name=str(dt)
                )
            ),
        )
        for f, dt in zip(output_df.columns, output_df.dtypes)
    ]

PandasTransformation.infer_features = __patched_infer_features

Note to reviewers
If you can point me to where I could add some feature repository tests to catch any regressions in the future that would be great 🙏

Fixes

Fixes #4308

tokoko · 2024-06-24T13:26:47Z

I think the best way for this to be tested would be in respective unit tests: test_on_demand_pandas_transformation.py, test_on_demand_python_transformation.py and test_substrait_transformation.py

alexmirrington · 2024-06-25T06:19:11Z

@tokoko Added some tests for lists of bools, strings, floats and ints for both python and pandas ODFVs, as well as a request source in there. LMK what you think!

sdk/python/feast/type_map.py

sdk/python/feast/transformation/python_transformation.py

alexmirrington · 2024-06-27T23:14:49Z

@HaoXuAI FYI seeing a few rate limits for bigtable and redshift in integration tests, doesn't seem related to my changes but maybe you know who best could take a look?

sdk/python/tests/unit/test_on_demand_pandas_transformation.py

tokoko · 2024-06-28T08:53:21Z

@alexmirrington Can you make a dummy commit to force a rerun?
@HaoXuAI Some tests still need First-time contributor approval

tokoko

LGTM

alexmirrington · 2024-07-02T13:18:23Z

Can you make a dummy commit to force a rerun?

@tokoko @HaoXuAI Just pushed some validation checks and tests as requested, should be good for final review now

sdk/python/feast/transformation/pandas_transformation.py

Signed-off-by: Alex Mirrington <[email protected]>

alexmirrington · 2024-07-07T04:06:03Z

Ready to merge if you are happy, just need permissions to run some final checks now that I've fixed linting issues.
cc @HaoXuAI

Fix OnDemandFeatureView type inference for array types Signed-off-by: Alex Mirrington <[email protected]>

# [0.40.0](v0.39.0...v0.40.0) (2024-07-31) ### Bug Fixes * Added missing type ([#4315](#4315)) ([86af60a](86af60a)) * Avoid XSS attack from Jinjin2's Environment(). ([#4355](#4355)) ([40270e7](40270e7)) * CGO Memory leak issue in GO Feature server ([#4291](#4291)) ([43e198f](43e198f)) * Deprecated the datetime.utcfromtimestamp(). ([#4306](#4306)) ([21deec8](21deec8)) * Fix SQLite import issue ([#4294](#4294)) ([398ea3b](398ea3b)) * Increment operator to v0.39.0 ([#4368](#4368)) ([3ddb4fb](3ddb4fb)) * Minor typo in the unit test. ([#4296](#4296)) ([6c75e84](6c75e84)) * OnDemandFeatureView type inference for array types ([#4310](#4310)) ([c45ff72](c45ff72)) * Remove redundant batching in PostgreSQLOnlineStore.online_write_batch and fix progress bar ([#4331](#4331)) ([0d89d15](0d89d15)) * Remove typo. ([#4351](#4351)) ([92d17de](92d17de)) * Retire the datetime.utcnow(). ([#4352](#4352)) ([a8bc696](a8bc696)) * Update dask version to support pandas 1.x ([#4326](#4326)) ([a639d61](a639d61)) * Update Feast object metadata in the registry ([#4257](#4257)) ([8028ae0](8028ae0)) * Using one single function call for utcnow(). ([#4307](#4307)) ([98ff63c](98ff63c)) ### Features * Add async feature retrieval for Postgres Online Store ([#4327](#4327)) ([cea52e9](cea52e9)) * Add Async refresh to Sql Registry ([#4251](#4251)) ([f569786](f569786)) * Add SingleStore as an OnlineStore ([#4285](#4285)) ([2c38946](2c38946)) * Add Tornike to maintainers.md ([#4339](#4339)) ([8e8c1f2](8e8c1f2)) * Bump psycopg2 to psycopg3 for all Postgres components ([#4303](#4303)) ([9451d9c](9451d9c)) * Entity key deserialization ([#4284](#4284)) ([83fad15](83fad15)) * Ignore paths feast apply ([#4276](#4276)) ([b4d54af](b4d54af)) * Move get_online_features to OnlineStore interface ([#4319](#4319)) ([7072fd0](7072fd0)) * Port mssql contrib offline store to ibis ([#4360](#4360)) ([7914cbd](7914cbd)) ### Reverts * Revert "fix: Avoid XSS attack from Jinjin2's Environment()." ([#4357](#4357)) ([cdeab48](cdeab48)), closes [#4355](#4355)

# [0.40.0](feast-dev/feast@v0.39.0...v0.40.0) (2024-07-31) ### Bug Fixes * Added missing type ([feast-dev#4315](feast-dev#4315)) ([86af60a](feast-dev@86af60a)) * Avoid XSS attack from Jinjin2's Environment(). ([feast-dev#4355](feast-dev#4355)) ([40270e7](feast-dev@40270e7)) * CGO Memory leak issue in GO Feature server ([feast-dev#4291](feast-dev#4291)) ([43e198f](feast-dev@43e198f)) * Deprecated the datetime.utcfromtimestamp(). ([feast-dev#4306](feast-dev#4306)) ([21deec8](feast-dev@21deec8)) * Fix SQLite import issue ([feast-dev#4294](feast-dev#4294)) ([398ea3b](feast-dev@398ea3b)) * Increment operator to v0.39.0 ([feast-dev#4368](feast-dev#4368)) ([3ddb4fb](feast-dev@3ddb4fb)) * Minor typo in the unit test. ([feast-dev#4296](feast-dev#4296)) ([6c75e84](feast-dev@6c75e84)) * OnDemandFeatureView type inference for array types ([feast-dev#4310](feast-dev#4310)) ([c45ff72](feast-dev@c45ff72)) * Remove redundant batching in PostgreSQLOnlineStore.online_write_batch and fix progress bar ([feast-dev#4331](feast-dev#4331)) ([0d89d15](feast-dev@0d89d15)) * Remove typo. ([feast-dev#4351](feast-dev#4351)) ([92d17de](feast-dev@92d17de)) * Retire the datetime.utcnow(). ([feast-dev#4352](feast-dev#4352)) ([a8bc696](feast-dev@a8bc696)) * Update dask version to support pandas 1.x ([feast-dev#4326](feast-dev#4326)) ([a639d61](feast-dev@a639d61)) * Update Feast object metadata in the registry ([feast-dev#4257](feast-dev#4257)) ([8028ae0](feast-dev@8028ae0)) * Using one single function call for utcnow(). ([feast-dev#4307](feast-dev#4307)) ([98ff63c](feast-dev@98ff63c)) ### Features * Add async feature retrieval for Postgres Online Store ([feast-dev#4327](feast-dev#4327)) ([cea52e9](feast-dev@cea52e9)) * Add Async refresh to Sql Registry ([feast-dev#4251](feast-dev#4251)) ([f569786](feast-dev@f569786)) * Add SingleStore as an OnlineStore ([feast-dev#4285](feast-dev#4285)) ([2c38946](feast-dev@2c38946)) * Add Tornike to maintainers.md ([feast-dev#4339](feast-dev#4339)) ([8e8c1f2](feast-dev@8e8c1f2)) * Bump psycopg2 to psycopg3 for all Postgres components ([feast-dev#4303](feast-dev#4303)) ([9451d9c](feast-dev@9451d9c)) * Entity key deserialization ([feast-dev#4284](feast-dev#4284)) ([83fad15](feast-dev@83fad15)) * Ignore paths feast apply ([feast-dev#4276](feast-dev#4276)) ([b4d54af](feast-dev@b4d54af)) * Move get_online_features to OnlineStore interface ([feast-dev#4319](feast-dev#4319)) ([7072fd0](feast-dev@7072fd0)) * Port mssql contrib offline store to ibis ([feast-dev#4360](feast-dev#4360)) ([7914cbd](feast-dev@7914cbd)) ### Reverts * Revert "fix: Avoid XSS attack from Jinjin2's Environment()." ([feast-dev#4357](feast-dev#4357)) ([cdeab48](feast-dev@cdeab48)), closes [feast-dev#4355](feast-dev#4355)

alexmirrington mentioned this pull request Jun 24, 2024

OnDemandFeatureView.feature_transformation.infer_features does pass UDF outputs to python_type_to_feast_value_type #4308

Closed

alexmirrington changed the title ~~Fix OnDemandFeatureView type inference for array types~~ fix: OnDemandFeatureView type inference for array types Jun 24, 2024

alexmirrington force-pushed the fix-odfv-type-inference branch from feac39c to 6b7afb9 Compare June 24, 2024 10:45

alexmirrington marked this pull request as ready for review June 24, 2024 10:49

alexmirrington force-pushed the fix-odfv-type-inference branch 3 times, most recently from 9594997 to 62208ea Compare June 25, 2024 06:18

HaoXuAI reviewed Jun 26, 2024

View reviewed changes

sdk/python/feast/type_map.py Outdated Show resolved Hide resolved

HaoXuAI added the ok-to-test label Jun 26, 2024

HaoXuAI reviewed Jun 26, 2024

View reviewed changes

sdk/python/feast/transformation/python_transformation.py Outdated Show resolved Hide resolved

tokoko reviewed Jun 28, 2024

View reviewed changes

sdk/python/tests/unit/test_on_demand_pandas_transformation.py Show resolved Hide resolved

tokoko approved these changes Jun 28, 2024

View reviewed changes

alexmirrington force-pushed the fix-odfv-type-inference branch 2 times, most recently from 92a3441 to 44ed226 Compare July 2, 2024 13:16

alexmirrington requested a review from HaoXuAI July 2, 2024 13:19

HaoXuAI approved these changes Jul 4, 2024

View reviewed changes

sdk/python/feast/transformation/pandas_transformation.py Show resolved Hide resolved

Fix OnDemandFeatureView type inference for array types

0ff73fb

Signed-off-by: Alex Mirrington <[email protected]>

alexmirrington force-pushed the fix-odfv-type-inference branch from 44ed226 to 0ff73fb Compare July 7, 2024 01:37

tokoko merged commit c45ff72 into feast-dev:master Jul 10, 2024

tsisodia10 pushed a commit to tsisodia10/feast that referenced this pull request Jul 23, 2024

fix: OnDemandFeatureView type inference for array types (feast-dev#4310)

e4edc11

Fix OnDemandFeatureView type inference for array types Signed-off-by: Alex Mirrington <[email protected]>

tsisodia10 pushed a commit to tsisodia10/feast that referenced this pull request Jul 23, 2024

fix: OnDemandFeatureView type inference for array types (feast-dev#4310)

ad6e723

Fix OnDemandFeatureView type inference for array types Signed-off-by: Alex Mirrington <[email protected]>

nick-amaya-sp pushed a commit to sailpoint/feast that referenced this pull request Jul 23, 2024

fix: OnDemandFeatureView type inference for array types (feast-dev#4310)

071dbd5

Fix OnDemandFeatureView type inference for array types Signed-off-by: Alex Mirrington <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: OnDemandFeatureView type inference for array types#4310

fix: OnDemandFeatureView type inference for array types#4310
tokoko merged 1 commit intofeast-dev:masterfrom
alexmirrington:fix-odfv-type-inference

alexmirrington commented Jun 24, 2024 •

edited

Loading

Uh oh!

tokoko commented Jun 24, 2024

Uh oh!

alexmirrington commented Jun 25, 2024

Uh oh!

Uh oh!

Uh oh!

alexmirrington commented Jun 27, 2024 •

edited

Loading

Uh oh!

Uh oh!

tokoko commented Jun 28, 2024

Uh oh!

tokoko left a comment

Uh oh!

alexmirrington commented Jul 2, 2024 •

edited

Loading

Uh oh!

Uh oh!

alexmirrington commented Jul 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

alexmirrington commented Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What and Why

Tests

Fixes

Uh oh!

tokoko commented Jun 24, 2024

Uh oh!

alexmirrington commented Jun 25, 2024

Uh oh!

Uh oh!

Uh oh!

alexmirrington commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

tokoko commented Jun 28, 2024

Uh oh!

tokoko left a comment

Choose a reason for hiding this comment

Uh oh!

alexmirrington commented Jul 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

alexmirrington commented Jul 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alexmirrington commented Jun 24, 2024 •

edited

Loading

alexmirrington commented Jun 27, 2024 •

edited

Loading

alexmirrington commented Jul 2, 2024 •

edited

Loading