Skip to content

TYP: resolve mypy errors in core/arrays/_ranges.py #52943

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
May 7, 2023
Merged

TYP: resolve mypy errors in core/arrays/_ranges.py #52943

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7719d89
Cast return values to int
jlge Apr 26, 2023
04a39bf
Remove mypy error comment
jlge Apr 28, 2023
46688e2
Add return statement to function
jlge Apr 28, 2023
77ff3ba
Change function signatures
jlge May 7, 2023
79afd7b
Add type annotation to variables
jlge May 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions pandas/core/arrays/_ranges.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,7 @@ def _generate_range_overflow_safe_signed(
# Putting this into a DatetimeArray/TimedeltaArray
# would incorrectly be interpreted as NaT
raise OverflowError
# error: Incompatible return value type (got "signedinteger[_64Bit]",
# expected "int")
return result # type: ignore[return-value]
return int(result)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the result incorrect, or the type-hint incorrect? In general, fixing type-hints shouldn't change behavior. But if this function is supposed to return a Python integer then this is okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function signature is this:

def _generate_range_overflow_safe_signed(
    endpoint: int, periods: int, stride: int, side: str
) -> int:

which I interpreted as the function returning a Python integer, would that be correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is one possibility. The other possibility is that the signature is incorrect. Looking at the implementation, it seems clear to me this was meant to return a NumPy integer:

result = np.int64(endpoint) + addend

A look through the git blame shows this function having type-hints added after the initial implementation and prior to it being type-checked (likely because NumPy didn't have type-hints). With all this, it seems much more likely that the signature, and not the implementation, is incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that makes sense! I've pushed the changes I've made to the function—I was unsure about one thing: in the function _generate_range_overflow_safe line 160, the variable midpoint is assigned the result of recursively calling the function, and then repassed in as a parameter. I casted this to an integer to match the function signature, but I wasn't sure if this was the right way to go about it, or if the endpoint parameter was supposed to be an np.int64 as well. However, in the function, endpoint is casted to np.int64, so I assume that the function signature of it being a Python int is correct.

except (FloatingPointError, OverflowError):
# with endpoint negative and addend positive we risk
# FloatingPointError; with reversed signed we risk OverflowError
Expand All @@ -204,9 +202,7 @@ def _generate_range_overflow_safe_signed(
i64max = np.uint64(i8max)
assert result > i64max
if result <= i64max + np.uint64(stride):
# error: Incompatible return value type (got "unsignedinteger", expected
# "int")
return result # type: ignore[return-value]
return int(result)

raise OutOfBoundsDatetime(
f"Cannot generate range with {side}={endpoint} and periods={periods}"
Expand Down