Repository navigation
fix: Keep the dataframe index out of write_to_offline_store - #7160
anishmehta24 wants to merge 1 commit into
Conversation
pa.Table.from_pandas stores a non-default index as an __index_level_0__ column, so a filtered dataframe reached the offline store with one column too many and the file offline store rejected it. Signed-off-by: Anish Mehta <[email protected]>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| provider.ingest_df_to_offline_store.assert_called_once() | ||
|
|
||
|
|
||
| def test_write_to_offline_store_does_not_write_the_dataframe_index(): |
There was a problem hiding this comment.
🟡 Missing test return type annotation
The new test omits -> None, despite the repository's requirement to type-hint every Python function signature.
| def test_write_to_offline_store_does_not_write_the_dataframe_index(): | |
| def test_write_to_offline_store_does_not_write_the_dataframe_index() -> None: |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7160 +/- ##
=======================================
Coverage 52.45% 52.45%
=======================================
Files 459 459
Lines 58172 58172
Branches 8577 8577
=======================================
+ Hits 30515 30517 +2
+ Misses 25573 25572 -1
+ Partials 2084 2083 -1
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
What this PR does / why we need it:
write_to_offline_storebuilds its Arrow table withpa.Table.from_pandas(df). When the dataframe has a non-default index, for example after filtering rows, pyarrow adds the index as an__index_level_0__column, and the file offline store rejects the write because the columns no longer match the batch source:push()withPushMode.OFFLINEorONLINE_AND_OFFLINEgoes through the same path. This passespreserve_index=False, like the otherfrom_pandascalls in the offline stores already do.Which issue(s) this PR fixes:
None filed.
Checks
git commit -s)Testing Strategy
Misc
Added
test_write_to_offline_store_does_not_write_the_dataframe_indextotests/unit/test_unit_feature_store.py; it fails on master and passes with the change (the file: 7 passed). I only ran that file, not the full suite. Manually, against a localFileSourcerepo, writing a filtered dataframe now succeeds instead of raising, and a dataframe with a default index writes as before. ruff 0.16.10 check and format pass.