Skip to content

TYP: Made ffill_indexer arg-type ndarray #44893

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
Dec 28, 2021
Merged

TYP: Made ffill_indexer arg-type ndarray #44893

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
19abe82
made left_indexer into ndarray with asarray
nickleus27 Dec 15, 2021
81ac8bd
pre-commit
nickleus27 Dec 15, 2021
86c7d41
typing cast left_indexer, right_indexer, and raise TypeError for None
nickleus27 Dec 27, 2021
84e70e2
pre-commit
nickleus27 Dec 28, 2021
763ef57
Merge branch 'master' into merge_arg_type
nickleus27 Dec 28, 2021
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
15 changes: 6 additions & 9 deletions pandas/core/reshape/merge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,16 +1648,13 @@ def get_result(self) -> DataFrame:
right_join_indexer: np.ndarray | None

if self.fill_method == "ffill":
# error: Argument 1 to "ffill_indexer" has incompatible type
# "Optional[ndarray]"; expected "ndarray"
left_join_indexer = libjoin.ffill_indexer(
left_indexer # type: ignore[arg-type]
)
# error: Argument 1 to "ffill_indexer" has incompatible type
# "Optional[ndarray]"; expected "ndarray"
right_join_indexer = libjoin.ffill_indexer(
right_indexer # type: ignore[arg-type]
if left_indexer is None:
raise TypeError("left_indexer cannot be None")
Copy link
Member

Choose a reason for hiding this comment

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

is this ever reached? is it because the user passed something invalid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbrockmendel I was wondering the same thing so I could write a better error message. Tbh I am not sure. I need to spend some more time to see how we get to this point

Copy link
Member

Choose a reason for hiding this comment

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

OK. this probably shouldn't have been merged until this was sorted out. NBD bc it isn't actively harmful here, but id appreciate it if you'd follow up on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbrockmendel I am having a hard time finding how to get the exception to be thrown. I am working on it, but it seems it will take me a little time.

Copy link
Contributor Author

@nickleus27 nickleus27 Dec 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

it seems that this exception situation only happens when the fill_method=='ffill'.
Is the fill_method argument only in pandas.merge_ordered()?

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 fill_method argument only in pandas.merge_ordered()?

looks like it, yah. i dont know this part of the code particularly well so im just checking docstrings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbrockmendel i do not think we can ever get to the TyperError thrown on L1651 because the left_indexer is only None when L956 has self.right_index as True. But, pandas.merge_ordered() does not have a right_index argument. I might be misunderstanding what is happening but I do not understand how you can set the fill_method='ffill' that i think is only an argument for merge_ordered but it has no right_index argument?

Copy link
Member

Choose a reason for hiding this comment

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

I think you're right. In this case, the check should go right after the _get_join_info call since i dont think it depends on anything after it (double-check me on this pls)

If im looking at this correctly, i think we can also rule out the cases where either left_join_indexer or right_join_indexer is None?

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 think it is possible for left_indexer and right_indexer to be None, but not when fill_method == "ffill".

left_indexer, right_indexer = cast(np.ndarray, left_indexer), cast(
np.ndarray, right_indexer
)
left_join_indexer = libjoin.ffill_indexer(left_indexer)
right_join_indexer = libjoin.ffill_indexer(right_indexer)
else:
left_join_indexer = left_indexer
right_join_indexer = right_indexer
Expand Down