-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
DOC: Remove absolute urls from the docs #32539
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: Remove absolute urls from the docs #32539
Conversation
pandas/core/generic.py
Outdated
| To learn more about the frequency strings, please see `this link | ||
| <https://pandas.pydata.org/pandas-docs/stable/user_guide/timeseries.html#offset-aliases>`__. | ||
| To learn more about the frequency strings, please see this link: | ||
| :ref:`Timeseries <timeseries.offset_aliases>` |
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 be kept as is (or with updated url if needed), as the docstrings are otherwise not useful as plain text
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.
(and the same for the other changes below in .py files)
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.
Ok, Reverting these changes
28066af to
addffc1
Compare
addffc1 to
a6b9927
Compare
fbdd732 to
5239174
Compare
|
Hi @jorisvandenbossche , I have updated my PR, it would be great if you can review once again. Thanks. |
jorisvandenbossche
left a comment
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.
Some more cases where we need to keep the absolute url .. (in practice, it seems that mostly there were good reasons we were doing that)
| DataFrames can be filtered in multiple ways; the most intuitive of which is using | ||
| `boolean indexing <https://pandas.pydata.org/pandas-docs/stable/indexing.html#boolean-indexing>`_. | ||
| :ref:`Boolean indexing <indexing.boolean>` |
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.
| :ref:`Boolean indexing <indexing.boolean>` | |
| :ref:`boolean indexing <indexing.boolean>` |
| In [1]: df = pd.DataFrame({'one': [1., 2., 3.]}) | ||
| In [2]: df.two = [4, 5, 6] | ||
| UserWarning: Pandas doesn't allow Series to be assigned into nonexistent columns - see https://pandas.pydata.org/pandas-docs/stable/indexing.html#attribute_access |
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 revert this one as well? (it's in the output of code, which should still return the absolute url)
doc/source/user_guide/indexing.rst
Outdated
| See the documentation here: | ||
| https://pandas.pydata.org/pandas-docs/stable/indexing.html#deprecate-loc-reindex-listlike | ||
| :ref:`Deprecation doc <indexing.deprecate_loc_reindex_listlike>` |
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.
You can make the "here" the text of the reference (and remove the colon), I think.
| KeyError in the future, you can use .reindex() as an alternative. | ||
| See the documentation here: | ||
| https://pandas.pydata.org/pandas-docs/stable/indexing.html#deprecate-loc-reindex-listlike |
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 here
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 also code output that should be left alone
doc/source/whatsnew/v0.23.0.rst
Outdated
|
|
||
| Starting January 1, 2019, pandas feature releases will support Python 3 only. | ||
| See `Dropping Python 2.7 <https://pandas.pydata.org/pandas-docs/version/0.24/install.html#install-dropping-27>`_ for more. | ||
| See :ref:`Dropping Python 2.7 <install>` for more. |
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.
You will need to keep the absolute reference here as well, since this needs to refer to the versioned docs (that page doesn't exist anymore on the stable docs)
doc/source/whatsnew/v0.23.1.rst
Outdated
|
|
||
| Starting January 1, 2019, pandas feature releases will support Python 3 only. | ||
| See `Dropping Python 2.7 <https://pandas.pydata.org/pandas-docs/version/0.24/install.html#install-dropping-27>`_ for more. | ||
| See :ref:`Dropping Python 2.7 <install>` for more. |
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 here
doc/source/whatsnew/v0.23.2.rst
Outdated
|
|
||
| Starting January 1, 2019, pandas feature releases will support Python 3 only. | ||
| See `Dropping Python 2.7 <https://pandas.pydata.org/pandas-docs/version/0.24/install.html#install-dropping-27>`_ for more. | ||
| See :ref:`Dropping Python 2.7 <install>` for more. |
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 here
doc/source/whatsnew/v0.23.4.rst
Outdated
|
|
||
| Starting January 1, 2019, pandas feature releases will support Python 3 only. | ||
| See `Dropping Python 2.7 <https://pandas.pydata.org/pandas-docs/version/0.24/install.html#install-dropping-27>`_ for more. | ||
| See :ref:`Dropping Python 2.7 <install>` for more. |
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 here (and others below)
|
Hi @jorisvandenbossche , I have made the changes, it would be great if you can look into once again. Thanks. |
jorisvandenbossche
left a comment
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.
One last remaining comment!
| KeyError in the future, you can use .reindex() as an alternative. | ||
| See the documentation here: | ||
| https://pandas.pydata.org/pandas-docs/stable/indexing.html#deprecate-loc-reindex-listlike |
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 also code output that should be left alone
|
Sorry for the poor issue description from my side @ArkadeepAdhikari. Looks good, there is just one case that needs to be reverted. Also, if you can add to |
|
Hi @datapythonista, should I be adding the check to not keep |
|
Good point. Let's leave it for later, as we'll need to think whether it's worth validating it and how. |
Since there are actually cases where we have this url in doc/source (several ones we reverted here in the PR), and given that there were not that much occurences to fix, I personally don't think it is worth the effort to make a code check for this |
|
Hi @datapythonista, @jorisvandenbossche, thanks for the clarifications. I have reverted the final case. Please take a look again. Thanks. |
doc/source/user_guide/indexing.rst
Outdated
| See the documentation here: | ||
| https://pandas.pydata.org/pandas-docs/stable/indexing.html#deprecate-loc-reindex-listlike | ||
| See the documentation :ref:`here <indexing.deprecate_loc_reindex_listlike>` |
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 think this is also an output from pandas, not an actual link that we want to change, isn't it?
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.
Yes this looks like a similar case, @jorisvandenbossche can you confirm?
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.
Yes
|
Hi @datapythonista, I have updated my PR, please feel free to review once again. |
|
Thanks @ArkadeepAdhikari ! |
|
Thanks for reviewing @jorisvandenbossche, @datapythonista! |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff