-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: Support historical feature retrieval with start_date/end_date in RemoteOfflineStore #5703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…RemoteOfflineStore Signed-off-by: Aniket Paluskar <[email protected]>
Signed-off-by: Aniket Paluskar <[email protected]>
… entity_df in offline server Signed-off-by: Aniket Paluskar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! @aniketpalu can you please rebase to resolve conflicts ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle empty entity_df in offline server.
sdk/python/feast/offline_server.py
Outdated
| datetime.fromisoformat(command["start_date"]) | ||
| ) | ||
| end_date = utils.make_tzaware(datetime.fromisoformat(command["end_date"])) | ||
| entity_df = pd.DataFrame( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not need to create dummy entity_df at all. We should handle empty(entity_df=None) entity_df for and in offline_store.get_historical_features which we are returning in this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, its not needed to create mock dataframe with timestamps, just had to make sure that empty_df is being passed.
Signed-off-by: Aniket Paluskar <[email protected]>
Signed-off-by: Aniket Paluskar <[email protected]>
What this PR does / why we need it:
Problem
Root Cause
Changes
Which issue(s) this PR fixes:
Misc