-
-
Notifications
You must be signed in to change notification settings - Fork 129
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
GH1043 Update _typing.pyi: add "cross" option to JoinHow #1044
base: main
Are you sure you want to change the base?
Conversation
pandas-stubs/_typing.pyi
Outdated
JoinHow: TypeAlias = Literal["left", "right", "outer", "inner", "cross"] | ||
MergeHow: TypeAlias = JoinHow |
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.
This fix is not correct, because JoinHow
is used as the type of the join
argument in the align()
method.
The right fix is to change the typing of DataFrame.join()
to use the existing MergeHow
type.
You would then add a test like your example that shows that "cross"
is accepted as a value for the how
argument of DataFrame.join()
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.
Thank you for the quick feedback, I've done as you said.
df1.join(df2, how="cross") | ||
df1.join(df2, how="inner") | ||
df1.join(df2, how="outer") | ||
df1.join(df2, how="left") | ||
df1.join(df2, how="right") |
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.
Can you use the check(assert_type(...
pattern here?
assert_type()
to assert the type of any return valueI haven't added tests since JoinHow doesn't seem to be tested in any case. Should I add some? Merge tests unimpacted.