DOC: Clean sort_values and sort_index docstrings
State: closed

Fixes some capitalization

Labels:
name: Docs
name: Clean

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=h1) Report
> Merging [#24843](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/f4458c18287288562b21adece524fc1b046e9724?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/24843/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #24843   +/-   ##
=======================================
  Coverage   92.39%   92.39%           
=======================================
  Files         166      166           
  Lines       52378    52378           
=======================================
  Hits        48393    48393           
  Misses       3985     3985
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.81% <ø> (ø)` | :arrow_up: |
| #single | `42.9% <ø> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `96.62% <ø> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=footer). Last update [f4458c1...0f5e99f](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=h1) Report
> Merging [#24843](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/2fa0835737a0e3e111e893d30ed2f25b7249fd4b?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/24843/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #24843      +/-   ##
==========================================
- Coverage   92.38%   92.37%   -0.01%     
==========================================
  Files         166      166              
  Lines       52413    52388      -25     
==========================================
- Hits        48424    48396      -28     
- Misses       3989     3992       +3
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.8% <ø> (-0.02%)` | :arrow_down: |
| #single | `42.9% <ø> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `93.68% <ø> (ø)` | :arrow_up: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `96.63% <ø> (ø)` | :arrow_up: |
| [pandas/core/nanops.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvbmFub3BzLnB5) | `93.84% <0%> (-0.53%)` | :arrow_down: |
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `83.58% <0%> (-0.18%)` | :arrow_down: |
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.4% <0%> (-0.07%)` | :arrow_down: |
| [pandas/io/formats/format.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvZm9ybWF0LnB5) | `97.98% <0%> (-0.01%)` | :arrow_down: |
| [pandas/core/indexes/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9pbnRlcnZhbC5weQ==) | `95.25% <0%> (ø)` | :arrow_up: |
| [pandas/core/api.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXBpLnB5) | `100% <0%> (ø)` | :arrow_up: |
| [pandas/core/arrays/integer.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVnZXIucHk=) | `96.32% <0%> (ø)` | :arrow_up: |
| [pandas/core/arrays/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVydmFsLnB5) | `93.12% <0%> (ø)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/pandas-dev/pandas/pull/24843/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=footer). Last update [2fa0835...75750da](https://codecov.io/gh/pandas-dev/pandas/pull/24843?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@gfyoung yep that's part of the existing validator. Below is output from master for me:

<details>

```sh
################################################################################
############# Docstring (pandas.core.generic.NDFrame.sort_values)  #############
################################################################################

Sort by the values along either axis

Parameters
----------%(optional_by)s
axis : %(axes_single_arg)s, default 0
     Axis to be sorted
ascending : bool or list of bool, default True
     Sort ascending vs. descending. Specify list for multiple sort
     orders.  If this is a list of bools, must match the length of
     the by.
inplace : bool, default False
     if True, perform operation in-place
kind : {'quicksort', 'mergesort', 'heapsort'}, default 'quicksort'
     Choice of sorting algorithm. See also ndarray.np.sort for more
     information.  `mergesort` is the only stable algorithm. For
     DataFrames, this option is only applied when sorting on a single
     column or label.
na_position : {'first', 'last'}, default 'last'
     `first` puts NaNs at the beginning, `last` puts NaNs at the end

Returns
-------
sorted_obj : %(klass)s

Examples
--------
>>> df = pd.DataFrame({
...     'col1' : ['A', 'A', 'B', np.nan, 'D', 'C'],
...     'col2' : [2, 1, 9, 8, 7, 4],
...     'col3': [0, 1, 9, 4, 2, 3],
... })
>>> df
    col1 col2 col3
0   A    2    0
1   A    1    1
2   B    9    9
3   NaN  8    4
4   D    7    2
5   C    4    3

Sort by col1

>>> df.sort_values(by=['col1'])
    col1 col2 col3
0   A    2    0
1   A    1    1
2   B    9    9
5   C    4    3
4   D    7    2
3   NaN  8    4

Sort by multiple columns

>>> df.sort_values(by=['col1', 'col2'])
    col1 col2 col3
1   A    1    1
0   A    2    0
2   B    9    9
5   C    4    3
4   D    7    2
3   NaN  8    4

Sort Descending

>>> df.sort_values(by='col1', ascending=False)
    col1 col2 col3
4   D    7    2
5   C    4    3
2   B    9    9
0   A    2    0
1   A    1    1
3   NaN  8    4

Putting NAs first

>>> df.sort_values(by='col1', ascending=False, na_position='first')
    col1 col2 col3
3   NaN  8    4
4   D    7    2
5   C    4    3
2   B    9    9
0   A    2    0
1   A    1    1

################################################################################
################################## Validation ##################################
################################################################################

10 Errors found:
	Summary does not end with a period
	Parameters {by} not documented
	Parameter "axis" description should finish with "."
	Parameter "inplace" description should start with a capital letter
	Parameter "inplace" description should finish with "."
	Parameter "na_position" description should start with a capital letter
	Parameter "na_position" description should finish with "."
	The first line of the Returns section should contain only the type, unless multiple values are being returned
	Return value has no description
	flake8 error: E203 whitespace before ':' (2 times)
2 Warnings found:
	No extended summary found
	See Also section not found
```

</details>


Might as well get all of these cleaned up while touching
@WillAyd : If that's the case, wouldn't the CI have failed then?
I believe we have a whitelist of error codes at the moment. The issues with this docstring wouldn't trigger a CI failure due to that but should nonetheless get cleaned up as we expand the whitelist

https://github.com/pandas-dev/pandas/blob/402d5dbd455bf35b875ea3c57feeef3141eb2bf0/ci/code_checks.sh#L243

I think before we can validate this error code (capitalization of the parameters description), we still need to change the validation to not report false positives when a parameter description starts with a list, and we also need to fix lots of cases that are incorrect.
@datapythonista Yes, the `na_position` error for instance seems to be a false positive because the description starts with one of the options for that argument.  Also, I've noticed that a lot of `pandas` documentation seems to follow a convention that single sentence descriptions don't need to be ended with a period, so lots of docstrings would likely be flagged here
Hello @dsaxton! Thanks for updating the PR.





Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on January 26, 2019 at 00:59 Hours UTC
Can you tackle remaining issues?

```sh
5 Errors found:
	Summary does not end with a period
	Parameters {by} not documented
	The first line of the Returns section should contain only the type, unless multiple values are being returned
	Return value has no description
	flake8 error: E203 whitespace before ':' (2 times)
```

First two are pretty clear. I think the last three may all be resolved if you add a description to the return object
@WillAyd I got the first one, but I'm not sure how the others might work when the docstring is a generic one covering different classes (e.g., the `Series` doesn't have a `by` argument):

```
        """
        Sort by the values along either axis.

        Parameters
        ----------%(optional_by)s
        axis : %(axes_single_arg)s, default 0
             Axis to be sorted.
        ascending : bool or list of bool, default True
             Sort ascending vs. descending. Specify list for multiple sort
             orders.  If this is a list of bools, must match the length of
             the by.
        inplace : bool, default False
             If True, perform operation in-place.
        kind : {'quicksort', 'mergesort', 'heapsort'}, default 'quicksort'
             Choice of sorting algorithm. See also ndarray.np.sort for more
             information.  `mergesort` is the only stable algorithm. For
             DataFrames, this option is only applied when sorting on a single
             column or label.
        na_position : {'first', 'last'}, default 'last'
             `first` puts NaNs at the beginning, `last` puts NaNs at the end.

        Returns
        -------
        sorted_obj : %(klass)s
```
Hmm it doesn't look like the docstrings are actually shared though? Looks like Series is creating its own so I don't think edits here affect that (though some cleanup / consolidation could happen in a separate PR)

Side note - if you merge master instead of rebasing shouldn't need to force push. Makes review process easier as it maintains comments better
> Hmm it doesn't look like the docstrings are actually shared though? Looks like Series is creating its own so I don't think edits here affect that (though some cleanup / consolidation could happen in a separate PR)
> 
> Side note - if you merge master instead of rebasing shouldn't need to force push. Makes review process easier as it maintains comments better

Got it, will merge instead in the future
@WillAyd Added a tiny fix to the `Series` method.  Was there anything else you'd like to wrap into this PR?
Can you post the output of `scripts/validate_docstrings.py pandas.core.generic.NDFrame.sort_<FUNC>`?
`sort_values`:

```
################################################################################
################################## Validation ##################################
################################################################################

4 Errors found:
	Parameters {by} not documented
	The first line of the Returns section should contain only the type, unless multiple values are being returned
	Return value has no description
	flake8 error: E203 whitespace before ':' (2 times)
2 Warnings found:
	No extended summary found
	See Also section not found
```

`sort_index`:

```
################################################################################
################################## Validation ##################################
################################################################################

5 Errors found:
	Parameter "axis" has no description
	Parameter "ascending" type should use "bool" instead of "boolean"
	Parameter "inplace" description should finish with "."
	The first line of the Returns section should contain only the type, unless multiple values are being returned
	Return value has no description
3 Warnings found:
	No extended summary found
	See Also section not found
	No examples section found
```
Thanks! Can you fix remaining errors?
I'm not quite sure how to fix the "Returns" errors given the generic nature of the docstring (same for the optional `by`).  Also for `sort_index` the `axis` argument does seem to have a description, so I'm a bit confused on that one.  Any ideas?
There was one other error in `Series.sort_index`, but should be good now
Check the documentation for set_axis in the same module could mimic or even copy the returns and axis parts there from 

Sent from my iPhone

> On Jan 23, 2019, at 8:34 PM, Daniel Saxton <notifications@github.com> wrote:
> 
> There was one other error in Series.sort_index, but should be good now
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

Added a couple more fixes.  It looks though that the `set_axis` method is also giving an error:

```
################################################################################
################################## Validation ##################################
################################################################################

1 Errors found:
	The first line of the Returns section should contain only the type, unless multiple values are being returned
```

Here's the returns section:

```
        Returns
        -------
        renamed : %(klass)s or None
            An object of same type as caller if inplace=False, None otherwise.
```
(Force push there was just to fix a commit message typo)
> OK so the returns section couldn't be copied verbatim. We aren't using substitution so `%(klass)s` isn't valid.
> 
> FYI I think your issue is the `or None` piece (wrong in docstring you copied from) but would be moot point with suggested change.

Okay, I see what you mean about the `klass` part.  Isn't it true though that the methods return `None` when an in-place operation is done?
Valid point - feel free to keep the None then and we can ignore that issue / merge after update 

Sent from my iPhone

> On Jan 24, 2019, at 5:53 AM, Daniel Saxton <notifications@github.com> wrote:
> 
> OK so the returns section couldn't be copied verbatim. We aren't using substitution so %(klass)s isn't valid.
> 
> FYI I think your issue is the or None piece (wrong in docstring you copied from) but would be moot point with suggested change.
> 
> Okay, I see what you mean about the klass part. Isn't it true though that the methods return None when an in-place operation is done?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

Pushed another commit.  I noticed we were actually editing the `sort_index` returns section so I reworded "sorted values" as "sorted index."

Sorry, but did we want to make a similar change to `sort_values` regarding the `klass` substitution?  I think only the `DataFrame` will use this method?
Makes sense for consistency

Sent from my iPhone

> On Jan 24, 2019, at 6:09 PM, Daniel Saxton <notifications@github.com> wrote:
> 
> Pushed another commit. I noticed we were actually editing the sort_index returns section so I reworded "sorted values" as "sorted index."
> 
> Sorry, but did we want to make a similar change to sort_values regarding the klass substitution? I think only the DataFrame will use this method?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

I suppose its just worth moving these docstrings to the frame class, since they aren't actually used anywhere else. If that's the case certainly lgtm (and thanks for sticking through all the edits!)

@jreback - do I need to milestone this PR any particular way before merging? 
0.25.0


> I suppose its just worth moving these docstrings to the frame class, since they aren't actually used anywhere else. If that's the case certainly lgtm (and thanks for sticking through all the edits!)

@WillAyd NP, thanks for the help!
@dsaxton on second thought this looks good as is. We could still refine this by actually using substitution (knowing there's a slight difference in signature) but can do in a separate PR if you want

Thanks again!