DOC GH22897 Fix docstring of join in pandas/core/frame.py #22904

JustinZhengBC · 2018-09-30T07:22:17Z

closes DOC: Fix the docstring of join in pandas/core/frame.py #22897
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

#22900 was dealt with by adding an "import pandas as pd" line to the start of the examples section.

pep8speaks · 2018-09-30T07:22:22Z

Hello @JustinZhengBC! Thanks for submitting the PR.

There are no PEP8 issues in the file pandas/core/frame.py !

datapythonista

Great changes, thanks a lot @JustinZhengBC. I added some comments mainly about the formatting of orignial the docstring. If you can fix those too, that would be great.

datapythonista · 2018-09-30T09:38:52Z

pandas/core/frame.py

@@ -6440,6 +6440,8 @@ def append(self, other, ignore_index=False,
    def join(self, other, on=None, how='left', lsuffix='', rsuffix='',
             sort=False):
        """
+        Append columns of another DataFrame.


Can you replace append by join? I think in general they mean the same, but in this context may give the idea that the they are being added at the end of the DataFrame, or that they are not aligned.

datapythonista · 2018-09-30T09:39:47Z

pandas/core/frame.py

@@ -6449,31 +6451,31 @@ def join(self, other, on=None, how='left', lsuffix='', rsuffix='',
        other : DataFrame, Series with name field set, or list of DataFrame


Do you mind removing the with name field set from the types, and have it in the description instead?

datapythonista · 2018-09-30T09:41:32Z

pandas/core/frame.py

@@ -6449,31 +6451,31 @@ def join(self, other, on=None, how='left', lsuffix='', rsuffix='',
        other : DataFrame, Series with name field set, or list of DataFrame
            Index should be similar to one of the columns in this one. If a
            Series is passed, its name attribute must be set, and that will be
-            used as the column name in the resulting joined DataFrame
+            used as the column name in the resulting joined DataFrame.
        on : name, tuple/list of names, or array-like


In the types, we try to have just the Python (or numpy...) types, and with a specific format that we can at some point parse and validate. Could you replace it by something like str, list of str or array-like please?

datapythonista · 2018-09-30T09:42:03Z

pandas/core/frame.py

        rsuffix : string
-            Suffix to use from right frame's overlapping columns
+            Suffix to use from right frame's overlapping columns.
        sort : boolean, default False


Can you replace the type string by str and boolean by bool

pandas/core/frame.py

datapythonista · 2018-09-30T09:44:55Z

pandas/core/frame.py

@@ -6485,70 +6487,67 @@ def join(self, other, on=None, how='left', lsuffix='', rsuffix='',

        Examples
        --------
+        >>> import pandas as pd
+
        >>> caller = pd.DataFrame({'key': ['K0', 'K1', 'K2', 'K3', 'K4', 'K5'],


do you mind replacing the name caller by df? we use this convention everywhere, I think it makes sense here too.

datapythonista · 2018-09-30T09:46:02Z

pandas/core/frame.py

+        2  K2  A2   B2
+        3  K3  A3  NaN
+        4  K4  A4  NaN
+        5  K5  A5  NaN

        See also
        --------


Can you move the See Also section before the examples, use a capital A for Also, and add a period at the end of the merge description.

codecov · 2018-09-30T16:35:41Z

Codecov Report

Merging #22904 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #22904   +/-   ##
=======================================
  Coverage   92.18%   92.18%           
=======================================
  Files         169      169           
  Lines       50830    50830           
=======================================
  Hits        46860    46860           
  Misses       3970     3970

Flag	Coverage Δ
#multiple	`90.6% <ø> (ø)`	⬆️
#single	`42.37% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/frame.py	`97.2% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14598c6...05988d4. Read the comment docs.

JustinZhengBC · 2018-09-30T16:38:17Z

Alright, I believe I have taken care of all the other issues now.

datapythonista

Looks great, just couple of small things that I didn't see in the previous review, and I think we can merge this. Thanks!

datapythonista · 2018-09-30T16:39:38Z

pandas/core/frame.py

-            used as the column name in the resulting joined DataFrame
-        on : name, tuple/list of names, or array-like
+            used as the column name in the resulting joined DataFrame.
+        on : str, list of str, or array-like


can you add that it's optional: on : str, list of str, or array-like, optional

datapythonista · 2018-09-30T16:40:13Z

pandas/core/frame.py

+              of the calling's one.
+        lsuffix : str
+            Suffix to use from left frame's overlapping columns.
+        rsuffix : str


can you add the default: str, default ''

datapythonista · 2018-09-30T16:45:41Z

pandas/core/frame.py

@@ -6483,76 +6485,72 @@ def join(self, other, on=None, how='left', lsuffix='', rsuffix='',
        Support for specifying index levels as the `on` parameter was added
        in version 0.23.0


Do you mind capitalizing the first letter of each sentence, and ending them in period in the Notes. For the First paragraph, if you can add backticks around the parameters on, lsuffix... that would be great too. So: "Options `on`, `lsuffix` and `rsuffix` are not supported [...]"

datapythonista · 2018-09-30T16:47:14Z

pandas/core/frame.py

+        2  K2  A2   B2
+        3  K3  A3  NaN
+        4  K4  A4  NaN
+        5  K5  A5  NaN

        Returns
        -------


Sorry, I missed before that the Returns was here. Can you move it after the parameters, and use this format:

Returns ------- DataFrame A short description on what is being returned.

…ropriate

JustinZhengBC · 2018-09-30T17:31:28Z

I fixed those issues. I also added some backticks around parameter names in other sections, as that was what other docstrings seemed to be doing.

datapythonista · 2018-09-30T17:39:19Z

pandas/core/frame.py

+
+        Another option to join using the key columns is to use the `on`
+        parameter. DataFrame.join always uses `other`'s index but we can use
+        any column in ``df``. This method preserves the original DataFrame's


The double backticks is for code, and the single backticks for objects that are being referenced. We don't have an exact convention for things like the parameters or the variables. Can you leave them with single backticks for now, as it's inconsistent to have df with double, and the rest like other with single. Other than that, the PR looks great.

…rame.py" This reverts commit 8139490.

…s/core/frame.py"" This reverts commit 19725a4.

…nto DOC-22897

JustinZhengBC · 2018-09-30T18:05:15Z

I changed the backticks around the df's to be single again.

datapythonista

Looks great, thanks for the PR @JustinZhengBC

We'll merge it once the CI checks are on green. Hope to see more of your contributions.

datapythonista · 2018-11-03T07:12:49Z

Superseded by #23471

DOC GH22897 Fix docstring of join in pandas/core/frame.py

cf56698

datapythonista reviewed Sep 30, 2018

View reviewed changes

datapythonista added Docs Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Sep 30, 2018

DOC GH22897 Fix other formatting issues in pandas/core/frame.py

988c40c

datapythonista reviewed Sep 30, 2018

View reviewed changes

JustinZhengBC added 2 commits September 30, 2018 10:24

DOC GH22897 Fix additional formatting issues in pandas/core/frame.py

8139490

DOC GH22897 Modified previous commit to use double backticks when app…

762f66d

…ropriate

datapythonista reviewed Sep 30, 2018

View reviewed changes

JustinZhengBC added 4 commits September 30, 2018 10:56

Revert "DOC GH22897 Fix additional formatting issues in pandas/core/f…

19725a4

…rame.py" This reverts commit 8139490.

Revert "Revert "DOC GH22897 Fix additional formatting issues in panda…

32b475d

…s/core/frame.py"" This reverts commit 19725a4.

Merge branch 'DOC-22897' of https://github.com/JustinZhengBC/pandas i…

69f81dc

…nto DOC-22897

DOC GH22897 Undo previous commit

05988d4

datapythonista approved these changes Sep 30, 2018

View reviewed changes

datapythonista mentioned this pull request Oct 1, 2018

DOC: Fix the docstring of resample in pandas/core/generic.py #22894

Closed

JustinZhengBC mentioned this pull request Oct 5, 2018

CLN GH22875 Replace bare excepts by explicit excepts in pandas/io/ #23004

Merged

4 tasks

datapythonista mentioned this pull request Nov 3, 2018

DOC: Updating DataFrame.join docstring #23471

Merged

4 tasks

datapythonista closed this Nov 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC GH22897 Fix docstring of join in pandas/core/frame.py #22904

DOC GH22897 Fix docstring of join in pandas/core/frame.py #22904

JustinZhengBC commented Sep 30, 2018 •

edited

Loading

pep8speaks commented Sep 30, 2018

datapythonista left a comment

datapythonista Sep 30, 2018

datapythonista Sep 30, 2018

datapythonista Sep 30, 2018

datapythonista Sep 30, 2018

datapythonista Sep 30, 2018

datapythonista Sep 30, 2018

codecov bot commented Sep 30, 2018 •

edited

Loading

JustinZhengBC commented Sep 30, 2018

datapythonista left a comment

datapythonista Sep 30, 2018

datapythonista Sep 30, 2018

datapythonista Sep 30, 2018

datapythonista Sep 30, 2018

JustinZhengBC commented Sep 30, 2018

datapythonista Sep 30, 2018

JustinZhengBC commented Sep 30, 2018

datapythonista left a comment

datapythonista commented Nov 3, 2018

		@@ -6449,31 +6451,31 @@ def join(self, other, on=None, how='left', lsuffix='', rsuffix='',
		other : DataFrame, Series with name field set, or list of DataFrame

		@@ -6483,76 +6485,72 @@ def join(self, other, on=None, how='left', lsuffix='', rsuffix='',
		Support for specifying index levels as the `on` parameter was added
		in version 0.23.0

DOC GH22897 Fix docstring of join in pandas/core/frame.py #22904

DOC GH22897 Fix docstring of join in pandas/core/frame.py #22904

Conversation

JustinZhengBC commented Sep 30, 2018 • edited Loading

pep8speaks commented Sep 30, 2018

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 30, 2018 • edited Loading

Codecov Report

JustinZhengBC commented Sep 30, 2018

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JustinZhengBC commented Sep 30, 2018

Choose a reason for hiding this comment

JustinZhengBC commented Sep 30, 2018

datapythonista left a comment

Choose a reason for hiding this comment

datapythonista commented Nov 3, 2018

JustinZhengBC commented Sep 30, 2018 •

edited

Loading

codecov bot commented Sep 30, 2018 •

edited

Loading