8000 DOC Skip @property on classes in the auto generated API reference by betatim · Pull Request #30989 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

DOC Skip @property on classes in the auto generated API reference #30989

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 17, 2025
Merged

DOC Skip @property on classes in the auto generated API reference #30989

merged 2 commits into from
Mar 17, 2025

Conversation

betatim
Copy link
Member
@betatim betatim commented Mar 13, 2025

Reference Issues/PRs

Closes #30957

What does this implement/fix? Explain your changes.

This sets up a function to determine what should be automatically documented and what not. We skip over @property as these get documented manually in the Attributes section already.

Any other comments?

I asked perplexity how to do this and this is the suggestion. Let's see if this works 🤞

@github-actions GitHub Actions
Copy link
github-actions bot commented Mar 13, 2025 •
edited
Loading

✔️ Linting Passed

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

Generated for commit: 3bb77a1. Link to the linter CI: here

@betatim
Copy link
Member Author
betatim commented Mar 13, 2025

https://output.circle-artifacts.com/output/job/33a013c3-c6ab-48d1-af4e-cf1b3ee9523e/artifacts/0/doc/modules/generated/sklearn.svm.SVC.html#sklearn.svm.SVC :-o

thomasjpfan
thomasjpfan reviewed Mar 14, 2025
View reviewed changes
Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

There are some properties that are not "attributes", for example ConstantKernel.hyperparameters:

https://output.circle-artifacts.com/output/job/33a013c3-c6ab-48d1-af4e-cf1b3ee9523e/artifacts/0/doc/modules/generated/sklearn.gaussian_process.kernels.ConstantKernel.html#sklearn.gaussian_process.kernels.ConstantKernel

https://scikit-learn.org/stable/modules/generated/sklearn.gaussian_process.kernels.ConstantKernel.html#sklearn.gaussian_process.kernels.ConstantKernel.requires_vector_input

@betatim
Skip @propertys that represent fitted attributes
3bb77a1 
We can recognise fitted attributes based on their nameing convention
@betatim
Copy link
Member Author
betatim commented Mar 14, 2025

Of course there is legit uses of properties in auto docs :D

I changed the condition to look at the name of a property and skip it if it follows the convention for "fitted attributes". I thought about inspecting the class name that owns the property and having a list of classes and properties to skip for each class. Could go with that but it seems like maintaining the list would be "another thing to do" compared to inferring that we should hide a property based on its name.

WDYT?

thomasjpfan
thomasjpfan approved these changes Mar 14, 2025
View reviewed changes
Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@lesteve
Copy link
Member
lesteve commented Mar 17, 2025

Let's merge this one, thanks!

@lesteve lesteve merged commit 1bd24a2 into scikit-learn:main Mar 17, 2025
33 checks passed
@betatim betatim deleted the skip-property-auto-doc branch March 17, 2025 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasjpfan thomasjpfan thomasjpfan approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docs duplication between attributes and properties
3 participants
@betatim @lesteve @thomasjpfan
0