Skip to content

DOC: add missing links to introduction to pandas #32198

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

Merged
merged 3 commits into from
Feb 27, 2020

Conversation

raisadz
Copy link
Contributor

@raisadz raisadz commented Feb 23, 2020

I noticed that there were some links missing in `10 minutes to pandas'.

@@ -528,7 +528,7 @@ groups.
df.groupby('A').sum()

Grouping by multiple columns forms a hierarchical index, and again we can
apply the ``sum`` function.
apply the :meth:`~DataFrame.sum` function.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not correct, you need:: meth:`pandas.core.groupby.GroupBy.sum`

@jreback jreback added the Docs label Feb 23, 2020
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@@ -520,15 +520,15 @@ See the :ref:`Grouping section <groupby>`.
'D': np.random.randn(8)})
df

Grouping and then applying the :meth:`~DataFrame.sum` function to the resulting
Grouping and then applying the :meth:`~pandas.core.groupby.GroupBy.sum` function to the resulting
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the tilde required here? Can you build this to see how this currently renders?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no problems when I build the documentation locally. But when I pushed it to pandas-dev, some tests failed (previous commit). However, after I added tilde, all checks passed. I thought it might be because the link was displayed as pandas.core.groupby.GroupBy.sum() instead of sum().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it actually render the link? This is different than how we link to GroupBy methods in our what’s new so not sure this would really solve the problem at hand (could be wrong)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc the tilde is used to just print the last element in the link text. So, without the tilde the text link would be pandas.core.groupby.GroupBy.sum, and with the tilde should be just sum.

I don't think the doc build should fail in any of the cases. May be the error was something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it actually render the link?

Yes, it does render the link with and without the tilde.

iirc the tilde is used to just print the last element in the link text

Yes, it is correct. In the last commit the link is displayed as sum() instead of pandas.core.groupby.GroupBy.sum.

I don't think the doc build should fail in any of the cases. May be the error was something else?

I checked today locally, there were no problems with building of the documentation, so the error might be not related.

Should I push it without tilde? Or should I leave it like it is?

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless of the tilde discussion, this looks like a nice improvement, happy to get this merged either way.

@@ -520,15 +520,15 @@ See the :ref:`Grouping section <groupby>`.
'D': np.random.randn(8)})
df

Grouping and then applying the :meth:`~DataFrame.sum` function to the resulting
Grouping and then applying the :meth:`~pandas.core.groupby.GroupBy.sum` function to the resulting
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc the tilde is used to just print the last element in the link text. So, without the tilde the text link would be pandas.core.groupby.GroupBy.sum, and with the tilde should be just sum.

I don't think the doc build should fail in any of the cases. May be the error was something else?

@WillAyd WillAyd added this to the 1.1 milestone Feb 27, 2020
@WillAyd WillAyd merged commit 2c060b4 into pandas-dev:master Feb 27, 2020
@WillAyd
Copy link
Member

WillAyd commented Feb 27, 2020

Thanks @raisadz

roberthdevries pushed a commit to roberthdevries/pandas that referenced this pull request Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants