-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
Use api_reference as the source of truth for __all__ an __dir__ #30375
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
base: main
Are you sure you want to change the base?
Conversation
I just saw this, and looks pretty harmless and nice to have. Would you please resolve conflicts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a nit, otherwise LGTM. Not sure if we really want to cover the parts not covered in tests here.
@@ -27,7 +26,6 @@ | ||
"AdaBoostRegressor", | ||
"BaggingClassifier", | ||
"BaggingRegressor", | ||
"BaseEnsemble", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O_o, should we keep this?
Reference Issues/PRs
Follow up to #29793
What does this implement/fix? Explain your changes.
When using auto-complete on main I see:
This includes all the objects imported in the file including modules like
functools
. With this PR, I get this:Any other comments?
Using
api_reference
as the source of truth leads to some inconsistencies. There are items are not inapi_refernece
, but are in an existing__all__
. These are listed insklearn/tests/test_imports_public.py
asOBJECTS_NOT_IN_API_REFERENCE
. The possible causes are:I added some items into
api_reference
that seems like that should be documented in the first place.