-
Notifications
You must be signed in to change notification settings - Fork 91
Retrieve logical types passed to a component in the graph #3428
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
Codecov Report
@@ Coverage Diff @@
## main #3428 +/- ##
=======================================
+ Coverage 99.7% 99.7% +0.1%
=======================================
Files 332 332
Lines 32510 32545 +35
=======================================
+ Hits 32380 32415 +35
Misses 130 130
Continue to review full report at Codecov.
|
2e9c896
to
578362c
Compare
0c6fac5
to
42a9190
Compare
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!
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! Send it.
Dict - Mapping feature name to logical type instance. | ||
|
||
Raises: | ||
ValueError: If the component is not in the graph. |
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.
A hypernit - I'm not sure what the convention is if a function itself does not directly raise an exception. I feel like we can probably remove the Raises from here. If you take it to a logical extreme, you could include tons of nested exceptions if you don't limit it to just the immediate scope of the function. Feel free to just merge though, this is not a hill I will die on...or will I?
Pull Request Description
Fixes #3418
After creating the pull request: in order to pass the release_notes_updated check you will need to update the "Future Release" section of
docs/source/release_notes.rst
to include this pull request by adding :pr:123
.