-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: return common type in SuperComponent type compatibility check #9275
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
feat: return common type in SuperComponent type compatibility check #9275
Conversation
Pull Request Test Coverage Report for Build 15186561857Details
💛 - Coveralls |
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.
I've added a few suggestions to fix current type issues.
You also need to change the return type of _check_non_union_compatibility
and _is_compatible
to Tuple[bool, Optional[T]]:
.
Type checking should pass then!
LGTM, but would kindly ask to @sjrl for a second look |
@mdrazak2001 this is looking good! Just one comment about updating more of the unit tests, but otherwise should be good to go. |
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.
Thanks for your work on this!
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! I've left a minor nit, when done we should be able to merge! Thanks for your work!
…2001/haystack into feat/super-component-common-type
Related Issues
_is_compatible(type1, type2)
to return the common type #9076Proposed Changes:
How did you test it?
Notes for the reviewer
Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
and added!
in case the PR includes breaking changes.