-
Notifications
You must be signed in to change notification settings - Fork 29
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
refactor entities #586
refactor entities #586
Conversation
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
|
Co-authored-by: Viacheslav Kukushkin <[email protected]>
Co-authored-by: Viacheslav Kukushkin <[email protected]>
@VukW Regarding expected methods results type and intellisense, it seems to work when we do something like |
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.
LGTM
# Conflicts: # cli/cli_tests.sh
Auth test still fails because of |
Refactor entities and use inheritance to not repeat code. This is needed since for FL support we are adding 4 new entities with similar interface.
This PR also removes the complexity and notion of local-only for listing entities, and defines an
unregistered
notionOne downside for this change is intellisense in code editors is now broken. To solve this (in another PR), we can use type hints when retrieving entities, and refactor the inheritance of schemas.