TYP: narrow type bounds on extract_array #46942
Merged
Conversation
This file contains 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
iasoon
reviewed
May 4, 2022
twoertwein
reviewed
May 5, 2022
twoertwein
reviewed
May 5, 2022
| @@ -329,7 +337,8 @@ def array( | ||
| if dtype is None: | ||
| inferred_dtype = lib.infer_dtype(data, skipna=True) | ||
| if inferred_dtype == "period": | ||
| return PeriodArray._from_sequence(data, copy=copy) | ||
| period_data = cast(Union[Sequence[Optional[Period]], AnyArrayLike], data) |
There was a problem hiding this comment.
wouldn't mind a mypy ignore instead of the cast
There was a problem hiding this comment.
Do you prefer the ignore? I personally think the cast is more semantically correct, since we checked the type using lib.infer_dtype. This pattern also seems to be used in different parts of the codebase already, too ( example).
There was a problem hiding this comment.
I have no strong opinion (slightly in favor of ignore, unless it creates many follow up ignores) @simonjayhawkins has a better overview when to use casts I am/was a bit too liberal using them :)
twoertwein
reviewed
May 5, 2022
twoertwein
reviewed
May 5, 2022
|
@twoertwein @simonjayhawkins good here? |
twoertwein
approved these changes
May 12, 2022
mroeschke
approved these changes
May 25, 2022
|
Thanks @iasoon. #46942 (comment) can be followed up with in a subsequent PR if needed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
xref #37715
Narrowing the type bound allows resolving some ignored mypy errors.
The other modified code is needed because
extract_arrayno longer returnsAny, causing more strict type checking in the calling methods.