-
Notifications
You must be signed in to change notification settings - Fork 8
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
Chore: First class predictions #187
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.
This is already giving me peace of mind! 🧘
Seeing it all clearly in one spot did raise some questions. Let me know if any of it doesn't make sense. Happy to elaborate!
Nicer union syntax Co-authored-by: Cas Wognum <caswognum@outlook.com>
Co-authored-by: Cas Wognum <caswognum@outlook.com>
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.
Great work @cwognum and @kiramclean 🚀
I think this PR is very clean and adds much needed structure to prediction objects, thank you. I've added some comments but I don't think any are blocking.
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.
This looks very fine indeed. I had some suggestions, nothing blocking.
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! Suggested a fox for the build error.
Changelogs
Checklist:
feature
,fix
,chore
,documentation
ortest
(or ask a maintainer to do it for you).