Skip to content

Conversation

@raisadz
Copy link
Contributor

@raisadz raisadz commented Feb 23, 2020

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

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

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.

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