Visitar URL original
fix: Map INT32_LIST, PDF_BYTES and IMAGE_BYTES type names to their ValueType by raashish1601 · Pull Request #6969 · feast-dev/feast · GitHub
Skip to content

fix: Map INT32_LIST, PDF_BYTES and IMAGE_BYTES type names to their ValueType - #6969

Open
raashish1601 wants to merge 2 commits into
feast-dev:masterfrom
raashish1601:fix/value-type-str-mapping
Open

raashish1601 wants to merge 2 commits into
feast-dev:masterfrom
raashish1601:fix/value-type-str-mapping

Conversation

@raashish1601

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

_convert_value_type_str_to_value_type maps value type names to ValueType, falling back to STRING for unknown names. Three names fall back by mistake:

  • the INT32_LIST key is written as "INT32_LIST " (trailing space), so "INT32_LIST" never matches,
  • PDF_BYTES and IMAGE_BYTES are not in the map.
from feast.type_map import _convert_value_type_str_to_value_type
_convert_value_type_str_to_value_type("INT32_LIST")   # ValueType.STRING
_convert_value_type_str_to_value_type("PDF_BYTES")    # ValueType.STRING
_convert_value_type_str_to_value_type("IMAGE_BYTES")  # ValueType.STRING

The registry REST API uses this function to turn a field's valueType into a Feast type (api/registry/rest/feature_views.py, features.py, data_sources.py), so Array(Int32), PdfBytes and ImageBytes features are reported as String, including in the generated Field(name=..., dtype=...) code. The Snowflake UDF helper create_entity_dict uses it too.

This removes the trailing space and adds the two missing names.

Which issue(s) this PR fixes:

No issue, found while checking type name conversions for each ValueType.

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests

Added test_convert_value_type_str_round_trips_every_name in sdk/python/tests/unit/test_type_map.py, which checks every ValueType name; the INT32_LIST, PDF_BYTES and IMAGE_BYTES cases fail on master. The other tests of this function pass. ruff check and ruff format --check are clean on the changed files.

…lueType

The INT32_LIST key in _convert_value_type_str_to_value_type had a
trailing space, and PDF_BYTES and IMAGE_BYTES were missing, so all
three fell back to STRING. The registry REST API uses this to describe
feature types, so such features were reported (and rendered in the
generated Field code) as String.

Signed-off-by: Raashish Aggarwal <94279692+raashish1601@users.noreply.github.com>
@raashish1601
raashish1601 requested a review from a team as a code owner October 8, 2026 00:15
@codecov-commenter

codecov-commenter commented Oct 8, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.32%. Comparing base (9d42729) to head (c253e50).
⚠️ Report is 1 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6969   +/-   ##
=======================================
  Coverage   49.32%   49.32%           
=======================================
  Files         443      443           
  Lines       55094    55094           
  Branches     8017     8017           
=======================================
  Hits        27176    27176           
  Misses      26035    26035           
  Partials     1883     1883           
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 50.71% <ø> (ø)
Files with missing lines Coverage Δ
sdk/python/feast/type_map.py 64.35% <ø> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf930e5...c253e50. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@haoxu0 haoxu0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants