DOC: CategoricalIndex doc string

[topper-123]

Improvements to the doc string of CategoricalIndex .

[pep8speaks]

Hello @topper-123! Thanks for updating the PR.

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

Comment last updated on February 19, 2019 at 08:24 Hours UTC

[codecov]

Codecov Report

Merging #24852 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #24852      +/-   ##
==========================================
- Coverage   92.39%   92.39%   -0.01%     
==========================================
  Files         166      166              
  Lines       52391    52391              
==========================================
- Hits        48407    48406       -1     
- Misses       3984     3985       +1

Flag	Coverage Δ
#multiple	90.81% <ø> (ø)	⬆️
#single	42.9% <ø> (ø)	⬆️

Impacted Files	Coverage Δ
pandas/core/indexes/category.py	98.61% <ø> (ø)	⬆️
pandas/util/testing.py	88.04% <0%> (-0.1%)	⬇️

---

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 01e7872...aa16d5c. Read the comment docs.

[codecov]

Codecov Report

Merging #24852 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #24852      +/-   ##
==========================================
+ Coverage   91.73%   91.73%   +<.01%     
==========================================
  Files         173      173              
  Lines       52839    52839              
==========================================
+ Hits        48472    48473       +1     
+ Misses       4367     4366       -1

Flag	Coverage Δ
#multiple	90.29% <ø> (ø)	⬆️
#single	41.74% <ø> (+0.01%)	⬆️

Impacted Files	Coverage Δ
pandas/core/indexes/category.py	98.61% <ø> (ø)	⬆️
pandas/util/testing.py	87.66% <0%> (+0.09%)	⬆️

---

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 f74aba6...76970f3. Read the comment docs.

[WillAyd]

Can you also post the output of scripts/validate_docstrings.py for this?

[WillAyd]

Short summary should fit on one line

[topper-123]

Ok, made a shorter version.

[topper-123]

The validation output is

################################################################################
################################## Validation ##################################
################################################################################

3 Errors found:
        Parameters {fastpath} not documented
        Parameter "dtype" type should use "str" instead of "string"
        Examples do not pass tests

################################################################################
################################### Doctests ###################################
################################################################################

**********************************************************************
Line 73, in pandas.CategoricalIndex
Failed example:
    pd.CategoricalIndex(['a', 'b', 'c', 'a', 'b', 'c'])
Expected:
    CategoricalIndex(['a', 'b', 'c', 'a', 'b', 'c'], categories=['a', 'b', 'c'], ordered=False, dtype='category')  # noqa
Got:
    CategoricalIndex(['a', 'b', 'c', 'a', 'b', 'c'], categories=['a', 'b', 'c'], ordered=False, dtype='category')
**********************************************************************
Line 79, in pandas.CategoricalIndex
Failed example:
    pd.CategoricalIndex(c)
Expected:
    CategoricalIndex(['a', 'b', 'c', 'a', 'b', 'c'], categories=['a', 'b', 'c'], ordered=False, dtype='category')  # noqa
Got:
    CategoricalIndex(['a', 'b', 'c', 'a', 'b', 'c'], categories=['a', 'b', 'c'], ordered=False, dtype='category')
**********************************************************************
Line 87, in pandas.CategoricalIndex
Failed example:
    ci
Expected:
    CategoricalIndex(['a', 'b', 'c', 'a', 'b', 'c'], categories=['c', 'b', 'a'], ordered=True, dtype='category')  # noqa
Got:
    CategoricalIndex(['a', 'b', 'c', 'a', 'b', 'c'], categories=['c', 'b', 'a'], ordered=True, dtype='category')

1. Fastpath is deprecated, so it's ok not to add it to the doc string
2. The dtype "string" is a ok, as this is a free text not a type.
3. The examples do not pass the tests, because of the # noqa part. That is needed to pass flake8.

so I think those reported errors are aceptable?

[WillAyd]

For fast path best to document as a deprecated argument. For sure want to use str instead of string and can you just remove the #noqa directive? On quick glance didn't see the utility of it

[topper-123]

For fast path best to document as a deprecated argument.

fastpath isn't documented in any other instances where it's used, even where it's not deprecated. For example Series accepts a fastpath parameter, but it's not documented in its doc string.

For sure want to use str instead of string

Alright.

and can you just remove the #noqa directive? On quick glance didn't see the utility of it

The output from CategoricalIndex is >80 characters, so the linter complains about line length without the noqa. I don't like adding noqa either, but CategoricalIndex very fast becomes wider that 80 lines, so don't see how to avoid it using noqa, given how the repr of CategoricalIndex is set up currently.

IMO the line breaks for CategoricalIndex should be better enforced, so it's < pd.options.display.width. That could be an idea for a future PR?

[WillAyd]

OK fine with skipping fastpath for now; may be actually removed before we get full LINTing on docstrings anyway.

For CategoricalIndex repr you should see other examples within the module on how to show this which you can emulate

[WillAyd]

Can you merge master and address last comment?

[WillAyd]

Closing as stale; ping if you'd like to continue

[jorisvandenbossche]

If they are not give here (and also not in dtype), they will actually be inferred from the data

[jorisvandenbossche]

let's point here to the stable docs

[jorisvandenbossche]

@WillAyd since this is maybe not yet perfect, but still a significant improvement compared to status-quo, I would rather just merge it instead of closing, if that is fine with you.

For CategoricalIndex repr you should see other examples within the module on how to show this which you can emulate

The other docstring examples wrap the output over multiple lines. So that avoids the # noqa, but then does not pass the doctests I assume.
Doesn't matter too much which way we use here, but in any case: I would say this is a bug in the index repr (that it doesn't follow the max width), and not something that can be solved in this PR.

I merged master and maded some fixes.

[topper-123]

Had a single question, but otherwise I approve the fixes by @jorisvandenbossche. This will make the doc string better. That the repr is over 80 character is a seperate issue. I'll file an issue on that.

EDIT: An issue on this already exists at #12066.

[WillAyd]

Sorry missed the ping but sure agreed this is an improvement and can move forward. Thanks @topper-123 for the original PR and @jorisvandenbossche for continuing along