Update join docs for other param #46850

multimeric · 2022-04-23T10:33:35Z

Currently the DataFrame.join docs imply that the type of other is DataFrame | Series | list[DataFrame], but this is not correct, and list[Series | DataFrame] is a more appropriate type.

The type signature and also the method description have been updated to amend this.

closes #xxxx (Replace xxxx with the Github issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

Update join docs regarding using multiple Series

multimeric · 2022-04-24T01:59:45Z

No idea why this is failing 🤷

rhshadrach

Thanks for the PR! Are iterables of Series tested? If not, can you add a test for them.

pandas/core/frame.py

github-actions · 2022-05-26T00:05:33Z

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

rhshadrach · 2022-05-26T20:44:37Z

@multimeric - are you interested in continuing this?

multimeric · 2022-05-28T05:49:34Z

Hmm, I'm a bit stuck. I can't work out how to get mypy to understand it's working with a list of Series | DataFrame.

rhshadrach

Can you post the mypy issues you're seeing?

pandas/core/reshape/concat.py

multimeric · 2022-06-12T13:47:43Z

pandas/core/frame.py:9586: error: Argument 1 to "list" has incompatible type "Union[DataFrame, Series, Iterable[Union[DataFrame, Series]]]"; expected "Iterable[DataFrame]" [arg-type]

multimeric · 2022-06-12T13:52:12Z

It's not clear to me what type signature is causing this to fail. Even if I type cast it I'm still getting the same error.

…atch-1

rhshadrach · 2022-06-15T00:10:28Z

I think the line should be:

frames = [cast("DataFrame | Series", self)] + list(cast("Iterable[DataFrame | Series]", other))

You need quotes because otherwise the Python tries to actually use the | operator. In other words, the first argument of cast needs to be a valid Python object.
You need to cast [self] as well; mypy will object to combining lists of different types.

multimeric · 2022-06-15T03:27:53Z

Thanks, that fixed it. Indeed I didn't try casting both parts of the concatenation, partly because I would have thought mypy could have narrowed the type of self at that point to other to Iterable because of the if statements. Ah well!

…atch-1

datapythonista · 2022-06-16T05:17:59Z

@multimeric can you merge master into your branch, seems like the problem in the CI is unrelated.

multimeric · 2022-06-16T05:19:41Z

I actually did that just before. It looks like the current failure is just a timeout? Happy to pull again though.

…atch-1

multimeric · 2022-06-16T06:17:17Z

No, there is indeed a failure here. But it seems unrelated entirely.

rhshadrach

Glad it's working - two requests on the test.

cc @Dr-Irv for the change to the type-hints. Opened #29; plan to put up a PR there once this is merged.

pandas/tests/frame/methods/test_join.py

pandas/core/frame.py

Dr-Irv · 2022-06-16T21:02:01Z

pandas/core/frame.py

@@ -9582,7 +9582,7 @@ def join(

        Parameters
        ----------
-        other : DataFrame, Series, or list of DataFrame
+        other : DataFrame, Series, or list of either of these


I think this should say "or list of DataFrames, Series, or both". "Either of these" implies that it has to be all one type.

I went with "or a list containing any combination of DataFrames and Series", if that's okay

pandas/core/frame.py

…atch-1

Dr-Irv

One small thing to comment on cast

pandas/core/frame.py

github-actions · 2022-07-18T00:05:29Z

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

datapythonista · 2022-07-18T02:45:45Z

@multimeric do you have time to add the requested comment? And ir'd also be good if you can merge master into your branch

@rhshadrach I think your comments were addressed, can you check if this is ready from your side?

…atch-1

pandas/core/frame.py

rhshadrach

lgtm

rhshadrach · 2022-07-18T21:01:27Z

pandas/core/frame.py

@@ -9805,7 +9805,8 @@ def join(

        Parameters
        ----------
-        other : DataFrame, Series, or list of DataFrame
+        other : DataFrame, Series, or a list containing any combination of DataFrames
+            and Series.


code checks build is failing with

Error: /home/runner/work/pandas/pandas/pandas/core/frame.py:9789:PR08:pandas.DataFrame.join:Parameter "other" description should start with a capital letter

I think it's confused by the period here; hopefully removing it should resolve.

rhshadrach · 2022-07-19T21:13:11Z

pandas/core/frame.py

+        other : DataFrame, Series, or a list containing any combination of DataFrames
+            and Series


I see it now; the linter is complaining about the "and" being lowercase as if it's the start the description. I'd hazard a guess that sphinx does too. I think we should make the type more consise:

DataFrame, Series, or a list of any combination of these

pandas/core/frame.py

mroeschke · 2022-07-21T17:17:39Z

Thanks @multimeric for your persistence and all for the review!

multimeric added 2 commits April 23, 2022 20:32

Update join docs for other param

22c54af

Update join docs regarding using multiple Series

Update type for _join_compat

36eec5b

rhshadrach requested changes Apr 25, 2022

View reviewed changes

pandas/core/frame.py Show resolved Hide resolved

rhshadrach added Docs Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Apr 25, 2022

github-actions bot added the Stale label May 26, 2022

multimeric added 3 commits May 28, 2022 01:42

Allow any iterable for join; test join for a list of series

48048db

Update type signature

7811306

Update pd.concat type, add cast() to make frame.join() work with mypy

bb002f8

rhshadrach reviewed Jun 12, 2022

View reviewed changes

pandas/core/reshape/concat.py Outdated Show resolved Hide resolved

multimeric added 2 commits June 12, 2022 23:40

Fix type union syntax

436f5d3

NDFrame

76ead79

multimeric added 2 commits June 13, 2022 00:04

Remove cast

b12f42e

Merge branch 'main' of github.com:pandas-dev/pandas into multimeric-p…

e3c1fd3

…atch-1

Fix mypy errors

3b04d62

Merge branch 'main' of github.com:pandas-dev/pandas into multimeric-p…

f72bbdd

…atch-1

datapythonista removed the Stale label Jun 16, 2022

Merge branch 'main' of github.com:pandas-dev/pandas into multimeric-p…

7c1c113

…atch-1

rhshadrach requested changes Jun 16, 2022

View reviewed changes

pandas/tests/frame/methods/test_join.py Show resolved Hide resolved

pandas/tests/frame/methods/test_join.py Outdated Show resolved Hide resolved

Dr-Irv requested changes Jun 16, 2022

View reviewed changes

Dr-Irv reviewed Jun 16, 2022

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

multimeric added 3 commits June 17, 2022 13:05

Code review

0a241c0

Merge branch 'main' of github.com:pandas-dev/pandas into multimeric-p…

e83bace

…atch-1

Remove unnecessary assert

f72a5b5

Dr-Irv requested changes Jun 17, 2022

View reviewed changes

pandas/core/frame.py Show resolved Hide resolved

github-actions bot added the Stale label Jul 18, 2022

multimeric added 2 commits July 18, 2022 14:17

Merge branch 'main' of github.com:pandas-dev/pandas into multimeric-p…

6d17dc9

…atch-1

Add comment explaining the cast

75436c8

Dr-Irv approved these changes Jul 18, 2022

View reviewed changes

Dr-Irv reviewed Jul 18, 2022

View reviewed changes

pandas/core/frame.py Show resolved Hide resolved

Fix swapped order of cast comment

f48213f

rhshadrach approved these changes Jul 18, 2022

View reviewed changes

rhshadrach reviewed Jul 18, 2022

View reviewed changes

rhshadrach removed the Stale label Jul 18, 2022

rhshadrach added this to the 1.5 milestone Jul 18, 2022

Remove full stop

b79e66c

rhshadrach reviewed Jul 19, 2022

View reviewed changes

datapythonista reviewed Jul 21, 2022

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

Update pandas/core/frame.py

4ba0ce0

mroeschke approved these changes Jul 21, 2022

View reviewed changes

mroeschke merged commit 0ce2921 into pandas-dev:main Jul 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update join docs for other param #46850

Update join docs for other param #46850

multimeric commented Apr 23, 2022 •

edited

Loading

multimeric commented Apr 24, 2022

rhshadrach left a comment

github-actions bot commented May 26, 2022

rhshadrach commented May 26, 2022

multimeric commented May 28, 2022

rhshadrach left a comment

multimeric commented Jun 12, 2022 •

edited

Loading

multimeric commented Jun 12, 2022

rhshadrach commented Jun 15, 2022

multimeric commented Jun 15, 2022

datapythonista commented Jun 16, 2022

multimeric commented Jun 16, 2022

multimeric commented Jun 16, 2022

rhshadrach left a comment •

edited

Loading

Dr-Irv Jun 16, 2022

multimeric Jun 17, 2022

Dr-Irv left a comment

github-actions bot commented Jul 18, 2022

datapythonista commented Jul 18, 2022

rhshadrach left a comment

rhshadrach Jul 18, 2022

rhshadrach Jul 19, 2022

mroeschke commented Jul 21, 2022

		other : DataFrame, Series, or a list containing any combination of DataFrames
		and Series

Update join docs for other param #46850

Update join docs for other param #46850

Conversation

multimeric commented Apr 23, 2022 • edited Loading

multimeric commented Apr 24, 2022

rhshadrach left a comment

Choose a reason for hiding this comment

github-actions bot commented May 26, 2022

rhshadrach commented May 26, 2022

multimeric commented May 28, 2022

rhshadrach left a comment

Choose a reason for hiding this comment

multimeric commented Jun 12, 2022 • edited Loading

multimeric commented Jun 12, 2022

rhshadrach commented Jun 15, 2022

multimeric commented Jun 15, 2022

datapythonista commented Jun 16, 2022

multimeric commented Jun 16, 2022

multimeric commented Jun 16, 2022

rhshadrach left a comment • edited Loading

Choose a reason for hiding this comment

Dr-Irv Jun 16, 2022

Choose a reason for hiding this comment

multimeric Jun 17, 2022

Choose a reason for hiding this comment

Dr-Irv left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 18, 2022

datapythonista commented Jul 18, 2022

rhshadrach left a comment

Choose a reason for hiding this comment

rhshadrach Jul 18, 2022

Choose a reason for hiding this comment

rhshadrach Jul 19, 2022

Choose a reason for hiding this comment

mroeschke commented Jul 21, 2022

multimeric commented Apr 23, 2022 •

edited

Loading

multimeric commented Jun 12, 2022 •

edited

Loading

rhshadrach left a comment •

edited

Loading