DOC: Fixing some doc warnings #26786
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26786 +/- ##
==========================================
- Coverage 91.72% 91.72% -0.01%
==========================================
Files 178 178
Lines 50779 50779
==========================================
- Hits 46579 46575 -4
- Misses 4200 4204 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26786 +/- ##
==========================================
- Coverage 91.73% 91.73% -0.01%
==========================================
Files 178 178
Lines 50774 50774
==========================================
- Hits 46578 46575 -3
- Misses 4196 4199 +3
Continue to review full report at Codecov.
|
| @@ -10283,8 +10283,8 @@ def _doc_parms(cls): | |||
| Returns | |||
| ------- | |||
| %(name1)s or %(name2)s (if level specified)\ | |||
| %(see_also)s | |||
| %(examples)s\ | |||
| %(see_also)s\ | |||
There was a problem hiding this comment.
Do we have an expectation that substituted sections are responsible for line breaks? I feel like this has caused a few issues throughout code base curious if you have thoughts
There was a problem hiding this comment.
@datapythonista I might already have fixed this one in https://github.com/pandas-dev/pandas/pull/26780/files, but by adding a extra newline in the "see also" string instead of here.
My reasoning was that if you do it here, and no see also is introduced, you might get too many empty lines (but didn't try that actually, so not sure this is correct)
There was a problem hiding this comment.
This is trickier than what it looks, our validation complains if there are two line breaks together, and also a missing line break at the end. And we don't fail the validation, but we get warnings (and incorrect rendering) if the line break is missing before a section title, as the title is not detected.
It took me a while to find a working solution for all the cases, I think the one here is the only possible one, there are many docstrings affected by this.
There was a problem hiding this comment.
Yeah, I trust you here ;-) I only fixed that single case (of min/max I think), and didn't check the interaction with fixing others. But you might need to revert my change to get it working with master.
There was a problem hiding this comment.
I think the change you did was correct and is still needed, it just didn't address all the cases.
| @@ -10283,8 +10283,8 @@ def _doc_parms(cls): | |||
| Returns | |||
| ------- | |||
| %(name1)s or %(name2)s (if level specified)\ | |||
| %(see_also)s | |||
| %(examples)s\ | |||
| %(see_also)s\ | |||
There was a problem hiding this comment.
@datapythonista I might already have fixed this one in https://github.com/pandas-dev/pandas/pull/26780/files, but by adding a extra newline in the "see also" string instead of here.
My reasoning was that if you do it here, and no see also is introduced, you might get too many empty lines (but didn't try that actually, so not sure this is correct)
doc/source/whatsnew/v0.20.0.rst
Outdated
| @@ -396,7 +396,7 @@ IntervalIndex | |||
|
|
|||
| pandas has gained an ``IntervalIndex`` with its own dtype, ``interval`` as well as the ``Interval`` scalar type. These allow first-class support for interval | |||
| notation, specifically as a return type for the categories in :func:`cut` and :func:`qcut`. The ``IntervalIndex`` allows some unique indexing, see the | |||
| :ref:`docs <indexing.intervallindex>`. (:issue:`7640`, :issue:`8625`) | |||
| :ref:`docs <api.intervalindex>`. (:issue:`7640`, :issue:`8625`) | |||
There was a problem hiding this comment.
| :ref:`docs <api.intervalindex>`. (:issue:`7640`, :issue:`8625`) | |
| :ref:`docs <advanced.intervalindex>`. (:issue:`7640`, :issue:`8625`) |
I think the original intent was to refer to the user guide section, which is moved to "advanced.rst" (we actually shouldn't have renamed that label in the first place ..)
pandas/core/indexes/range.py
Outdated
| ---------- | ||
| start | ||
| stop | ||
| step |
There was a problem hiding this comment.
those are not actual attributes for what I saw, I'll revert here anyway, and let's take care of RangeIndex in a separate PR
There was a problem hiding this comment.
They recently got added as actual attributes, IIRC
There was a problem hiding this comment.
you're right, I think I didn't test in master
doc/source/reference/indexing.rst
Outdated
| @@ -493,3 +493,18 @@ Methods | |||
| PeriodIndex.asfreq | |||
| PeriodIndex.strftime | |||
| PeriodIndex.to_timestamp | |||
|
|
|||
| RangeIndex | |||
| ---------- | |||
There was a problem hiding this comment.
Looking more closely at the log output and our current docs -> RangeIndex is actually already included in the API docs in the "Numeric Index" above (I think it makes sense to have it in the numeric section)
But the question is then: why did we get those warnings in the current doc build on master. It might be that we need to add the attributes here as well (not fully sure)
|
@jorisvandenbossche addressed comments, I think this one should be ready. Are you working on other doc warnings? |
Not at the moment |
|
Down to 147 warnings :) |
git diff upstream/master -u -- "*.py" | flake8 --diffFixing some docs warnings, still many more, but will open few PRs with fixes, try to get a clean build and start failing the CI for doc warnings.