Skip to content
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

CLEAN: remove nonpublic stubs #170

Merged
merged 2 commits into from
Jul 27, 2022
Merged

CLEAN: remove nonpublic stubs #170

merged 2 commits into from
Jul 27, 2022

Conversation

Dr-Irv
Copy link
Collaborator

@Dr-Irv Dr-Irv commented Jul 26, 2022

  • Tests added
    • added test_pandas.py:test_unique()

Removals

  • pd.value_counts() - while exported, it is not documented
  • Everything in algorithms.pyi except for factorize() and unique()
  • pd.api.extensions.take - not documented
  • From pandas.core: apply.pyi, sorting.pyi, nanops.pyi - nothing there is public

Refinements

  • pd.unique() - added tests and overloads based on the docs

@Dr-Irv Dr-Irv requested a review from twoertwein July 26, 2022 02:05
@twoertwein
Copy link
Member

twoertwein commented Jul 26, 2022

  • pd.value_counts() - while exported, it is not documented

I guess we follow a "remove first, wait for user issue, create doc issue at pandas, add back"-approach?

edit: might not be the best user-experience but it is probably the best way to expand the pandas docs.

@twoertwein twoertwein requested a review from bashtage July 26, 2022 02:42
@Dr-Irv
Copy link
Collaborator Author

Dr-Irv commented Jul 26, 2022

  • pd.value_counts() - while exported, it is not documented

I guess we follow a "remove first, wait for user issue, create doc issue at pandas, add back"-approach?

edit: might not be the best user-experience but it is probably the best way to expand the pandas docs.

I will raise an issue in the pandas repo to ask if this is a docs oversight, or a coding error. Either it is not supposed to be public, so the code should get fixed, or the docs need to get fixed. If the former, then the change here is fine. If the latter, then most likely no user is going to complain about it until we document it, so removing it now causes no harm.

@Dr-Irv
Copy link
Collaborator Author

Dr-Irv commented Jul 26, 2022

I will raise an issue in the pandas repo to ask if this is a docs oversight, or a coding error. Either it is not supposed to be public, so the code should get fixed, or the docs need to get fixed. If the former, then the change here is fine. If the latter, then most likely no user is going to complain about it until we document it, so removing it now causes no harm.

Based on this: https://www.programcreek.com/python/example/101338/pandas.value_counts
it seems that it is meant to be public, and just missing from the docs, given how much it is used.

I will put it back in here.

Copy link
Member

@twoertwein twoertwein left a comment

Choose a reason for hiding this comment

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

Looks good to me

@twoertwein twoertwein merged commit 74f2b5a into pandas-dev:main Jul 27, 2022
@twoertwein
Copy link
Member

Thanks @Dr-Irv !

(When in doubt, I wouldn't mind removing slightly more than necessary: in the worst case we need to add it back but it will slowly make the pandas doc more complete.)

@Dr-Irv Dr-Irv deleted the deadtypes branch December 28, 2022 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants