Skip to content

DOC: Do not mention private classes in the documentation #26981

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
datapythonista opened this issue Jun 21, 2019 · 5 comments · Fixed by #26997
Closed

DOC: Do not mention private classes in the documentation #26981

datapythonista opened this issue Jun 21, 2019 · 5 comments · Fixed by #26997

Comments

@datapythonista
Copy link
Member

The public documentation shouldn't mention private classes, which the users are not expected to know about, and they could be removed in the future without proper deprecation.

We've got a script to find them, it can be run with ./scripts/validate_docstrings.py --errors=GL04

Currently there are those cases:

pandas.Series.pipe: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.Series.where: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.Series.mask: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.Series.first_valid_index: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.Series.last_valid_index: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.Series.tshift: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.Series.to_dense: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.core.resample.Resampler.get_group: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.DataFrame.where: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.DataFrame.mask: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.DataFrame.pipe: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.DataFrame.tshift: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.DataFrame.first_valid_index: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.DataFrame.last_valid_index: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.DataFrame.to_dense: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.core.groupby.GroupBy.get_group: Private classes (NDFrame) should not be mentioned in public docstrings
pandas.core.groupby.DataFrameGroupBy.tshift: Private classes (NDFrame) should not be mentioned in public docstrings

I'd say in most cases we can replace NDFrame by Series or DataFrame but may be we can rephrase sentences too.

After all them are corrected, we should add to ci/code_checks.sh the code GL04 to the part where we validate the documentation, so the CI checks for it, and mentions to NDFrame or other private classes are not reintroduced. Also in the same PR we can add GL05, which is not validated but there are no errors in the code for it, so we can start validating it.

@LiXuanqi
Copy link
Contributor

@datapythonista Can I take this issue?

@datapythonista
Copy link
Member Author

Yes please, go for it. Thanks!

@LiXuanqi
Copy link
Contributor

Hi @datapythonista , I have pulled a request. This is my first time to do the code review on the Github and found there is no reviewer in my request. Do I need to find someone to review my code or just wait?

@datapythonista
Copy link
Member Author

You just need to wait, we'll review when we're free. I see you replaced NDFrame by DataFrame. That's a bit misleading, since the referred objects can also be Series (NDFrame is the parent class of both Series and DataFrame). Sorry if that wasn't clear in the issue description. Can you have a look and make sure we're not having inaccurate information with the change please? Other than that looks great.

@LiXuanqi
Copy link
Contributor

LiXuanqi commented Jun 22, 2019

@datapythonista , Sorry for the misunderstanding, I will fix that tomorrow. Replace with Series or DataFrame. Thanks for suggestions.

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

Successfully merging a pull request may close this issue.

2 participants