-
-
Notifications
You must be signed in to change notification settings - Fork 19.2k
DOC: Update Sphinx Deprecated Directive #16512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DOC: Update Sphinx Deprecated Directive #16512
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16512 +/- ##
=======================================
Coverage 90.43% 90.43%
=======================================
Files 161 161
Lines 51045 51045
=======================================
Hits 46161 46161
Misses 4884 4884
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16512 +/- ##
=======================================
Coverage 90.43% 90.43%
=======================================
Files 161 161
Lines 51045 51045
=======================================
Hits 46161 46161
Misses 4884 4884
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #16512 +/- ##
=======================================
Coverage 90.93% 90.93%
=======================================
Files 161 161
Lines 49280 49280
=======================================
Hits 44814 44814
Misses 4466 4466
Continue to review full report at Codecov.
|
doc/source/io.rst
Outdated
| DEPRECATED: this argument will be removed in a future version. Please call | ||
| ``pd.read_csv(...).to_records()`` instead. | ||
| .. deprecated:: 0.18.2 | ||
| This argument will be removed in a future version. Please call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would eliminate things like: This argument will be removed in a future version. (look for occurences in the changes you made)
as the deprecated tag already has this (I think)
| skip_footer : int, default ``0`` | ||
| DEPRECATED: use the ``skipfooter`` parameter instead, as they are identical | ||
| .. deprecated:: 0.19.0 | ||
| Use the ``skipfooter`` parameter instead, as they are identical |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g. this like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments.
Can you show how it looks like when rendered in html with sphinx? (eg screenshot)
Main comment is that I am not sure it is a good idea to do this conversion when the 'DEPRECATED' is the first sentence of a docstring (but to be sure, we should actually test what the api summary tables would look like when using it)
doc/sphinxext/numpydoc/README.rst
Outdated
| each entry to have a separate page. | ||
|
|
||
| - numpydoc_edit_link: bool (DEPRECATED -- edit your HTML template instead) | ||
| - numpydoc_edit_link: bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please leave this one as is (numpydoc's source is just included in pandas, we should adapt it only minimally)
| as_recarray : boolean, default ``False`` | ||
| DEPRECATED: this argument will be removed in a future version. Please call | ||
| ``pd.read_csv(...).to_records()`` instead. | ||
| .. deprecated:: 0.18.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using the deprecated directive, you will need to add blank lines around it
pandas/_libs/tslib.pyx
Outdated
| """ | ||
| DEPRECATED: use :meth:`to_pydatetime` instead. | ||
| .. deprecated:: 0.19.0 | ||
| Use :meth:`to_pydatetime` instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you leave this one as is? The thing is that the first sentence of the docstring gets added to the API overview. So the actual "DEPRECATED .. " is then more useful as the directive code (although I am actually not sure what sphinx would put in the api summary table)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pandas/core/frame.py
Outdated
| """ | ||
| DEPRECATED: use :meth:`DataFrame.sort_index` | ||
| .. deprecated:: 0.20.0 | ||
| Use :meth:`DataFrame.sort_index` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above for this one, and some others below as well
|
can you rebase and update? |
|
My apologies for the late update - been traveling with limited wifi access. Updated per comments to avoid losing information in the API summary table when using sphinx directives. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates!
| def convert_objects(self, convert_dates=True, convert_numeric=False, | ||
| convert_timedeltas=True, copy=True): | ||
| """ | ||
| Deprecated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one should not be removed I think
pandas/core/strings.py
Outdated
| na : default NaN, fill value for missing values. | ||
| as_indexer : DEPRECATED | ||
| as_indexer : False, by default, gives deprecated behavior better | ||
| achieved using str_extract. True return boolean indexer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not fully correct. It is just ignored, so the explanation for True or False doesn't matter.
pandas/io/sql.py
Outdated
| DEPRECATED: this parameter will be removed in a future version | ||
| .. deprecated:: 0.19.0 | ||
| 'sqlite' is the only supported option if SQLAlchemy is not | ||
| installed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'installed' is not fully correct, as you can use the sqlite backend also when sqlalchemy is installed. So I would make 'used' of it.
(the same for all the other occurrences of this one both in this file as in generic.py)
| @@ -1,1185 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you deleted this file by accident?
pandas/core/generic.py
Outdated
| """ | ||
| DEPRECATED: consolidate will be an internal implementation only. | ||
| .. deprecated:: 0.20.0 | ||
| Consolidate will be an internal implementation only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here as previous. By putting this deprecated directive on the first line, it gets less informative in a api summary table
pandas/core/series.py
Outdated
| def sortlevel(self, level=0, ascending=True, sort_remaining=True): | ||
| """ | ||
| DEPRECATED: use :meth:`Series.sort_index` | ||
| .. deprecated:: 0.20.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one I would leave as well
|
@GuessWhoSamFoo Thanks! |

git diff upstream/master --name-only -- '*.py' | flake8 --diffXR #6581 for version numbers. Hopefully whitespace is okay.