-
Notifications
You must be signed in to change notification settings - Fork 44
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
[FSTORE-1202] Support similarity search in external fg #1210
[FSTORE-1202] Support similarity search in external fg #1210
Conversation
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.
Please document the not straightforward behaviour of the .read method for fg has an embedding column and online=True. It should appear in both the docstring and I would suggest the error message as well. Point towards the show method in this case
python/hsfs/feature_group.py
Outdated
self, | ||
dataframe_type: Optional[str] = "default", | ||
online: Optional[bool] = False, | ||
read_options: Optional[dict] = {}, |
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.
read_options should default to None and get initialised in the function body to make sonarlint happy :). Additionally read_options is not used in the function, did you forget to add it to the query.read method?
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, smart thinking to move splitting the ux decision to a different JIRA to move forward and get the PR merged
This PR adds/fixes/changes...
JIRA Issue: -
Priority for Review: -
Related PRs: -
How Has This Been Tested?
Checklist For The Assigned Reviewer: