Skip to content

DOC: Updating capitalization in folder doc/source/reference #32824

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

Closed
wants to merge 24 commits into from
Closed

DOC: Updating capitalization in folder doc/source/reference #32824

wants to merge 24 commits into from

Conversation

cleconte987
Copy link
Contributor

@cleconte987 cleconte987 commented Mar 19, 2020

@WillAyd
Copy link
Member

WillAyd commented Mar 19, 2020

@datapythonista

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.

Thanks for working on this @cleconte987, looks great.

What would be very useful is to add the files that are fixed to https://github.com/pandas-dev/pandas/blob/master/ci/code_checks.sh#L330, so no more errors are being introduced in those fails (the CI will fail and let us know if any PR introduces titles capitalized incorrectly).

If you can add it to this PR that would be great.

@cleconte987
Copy link
Contributor Author

cleconte987 commented Mar 19, 2020 via email

@simonjayhawkins simonjayhawkins added this to the 1.1 milestone Mar 20, 2020
@datapythonista
Copy link
Member

Thanks @cleconte987, changes look great. But looks like there are still several errors in that directory. Most of them need to be added here: https://github.com/pandas-dev/pandas/blob/master/scripts/validate_rst_title_capitalization.py#L20

Do you mind adding them, and fixing the cases that need it, if any? Thanks!

@datapythonista datapythonista changed the title Updating capitalization in folder doc/source/reference DOC: Updating capitalization in folder doc/source/reference Mar 20, 2020
@pep8speaks
Copy link

pep8speaks commented Mar 23, 2020

Hello @cleconte987! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-03-27 07:48:21 UTC

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.

Really nice, not sure about one of the cases, but other than that looks great.

@@ -110,7 +110,7 @@ Binary operator functions
Series.product
Series.dot

Function application, groupby & window
Function application, GroupBy & window
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for this one? Just my opinion, but I think this one was correct, and we don't want to capitalize groupy to GroupBy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might have wrongly seen it with capital letters or something, re-updating PR

Copy link
Member

Choose a reason for hiding this comment

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

I see your point. to me it feels that there is the class GroupBy, but here it's not talking about the class, but about the concept. I guess we can use group by or groupping by to disambiguate. But not that important, we can merge it like this and change it later if we think that makes sense.

Merge remote-tracking branch 'upstream_main_pandas/master' into changes_to_pandas_remote
@cleconte987
Copy link
Contributor Author

cleconte987 commented Mar 25, 2020

Ok.. again failures in the PR sorry for that. I know now why I putted "GroupBy" as an exception. Because it is written like that in title in file doc/source/reference/groupby.rst
I will re-update the PR. Should I add it to exceptions or not? I think it should

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.

Thanks, nice work.

@@ -110,7 +110,7 @@ Binary operator functions
Series.product
Series.dot

Function application, groupby & window
Function application, GroupBy & window
Copy link
Member

Choose a reason for hiding this comment

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

I see your point. to me it feels that there is the class GroupBy, but here it's not talking about the class, but about the concept. I guess we can use group by or groupping by to disambiguate. But not that important, we can merge it like this and change it later if we think that makes sense.

@cleconte987
Copy link
Contributor Author

Thank you.

Yes I understand now for the case of "groupby/GroupBy", but yeah it was not passing the tests then. Ok :)

@simonjayhawkins
Copy link
Member

@cleconte987 can you merge upstream master to resolve conflicts

@datapythonista
Copy link
Member

I guess everything that this PR was proposing has already been fixed in master, since no changes appear in the diff. Closing, let me know if that's not correct @cleconte987, happy to reopen this is the problem is another one.

@cleconte987
Copy link
Contributor Author

cleconte987 commented Apr 6, 2020

No everything was updated, indeed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants