Python: Remove mypy error #4728
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fokko
commented
May 9, 2022
| raise NotImplementedError() | ||
|
|
||
| def apply(self, value: S) -> Optional[int]: | ||
| if value is None: |
Contributor
Author
There was a problem hiding this comment.
Since we have S, it can never be None, otherwise, the signature should be Optional[S]. If we want, we can still check if it None, "" or [] using:
if not value:I've removed it for now, because according to the current definition it is unreachable code.
Contributor
Author
There was a problem hiding this comment.
Okay, looking at the tests, we might expect a None there. I've changed the code the other way around.
Contributor
There was a problem hiding this comment.
Yeah, transforms can be applied to optional columns so this should be Optional[S].
Fokko
force-pushed
the
fd-fd-tox-error
branch
2 times, most recently
from
c615ced to
1320903
Compare
Merged
Mypy complaints about: No [mypy] section in config file Also enabled some additional checks on unreachable code
rdblue
reviewed
May 10, 2022
| return "null" | ||
| return str(value) | ||
| def to_human_string(self, value: Optional[S]) -> str: | ||
| return str(value) if value is not None else "null" |
Contributor
There was a problem hiding this comment.
Is this change necessary for this PR? We generally try to avoid code churn that isn't making functional changes.
rdblue
approved these changes
May 10, 2022
Contributor
|
Thanks, @Fokko! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mypy complaints about the config file:
Also enabled some additional checks on unreachable code