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

Use api_reference as the source of truth for __all__ an __dir__ #30375

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented Nov 29, 2024

Reference Issues/PRs

Follow up to #29793

What does this implement/fix? Explain your changes.

When using auto-complete on main I see:

Screenshot 2024-11-28 at 5 42 15 PM

This includes all the objects imported in the file including modules like functools. With this PR, I get this:

Screenshot 2024-11-28 at 5 51 48 PM

Any other comments?

Using api_reference as the source of truth leads to some inconsistencies. There are items are not in api_refernece, but are in an existing __all__. These are listed in sklearn/tests/test_imports_public.py as OBJECTS_NOT_IN_API_REFERENCE. The possible causes are:

  1. A submodule (This is okay)
  2. Reimported from somewhere else, and is documented there (This is also okay, but a bit weird to have two import locations)
  3. Importable but not documented
  4. Deprecated

I added some items into api_reference that seems like that should be documented in the first place.

Copy link

github-actions bot commented Nov 29, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 112d63c. Link to the linter CI: here

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.

1 participant