Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

DOC Update docstring in partial_dependence.py #31309

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

Merged
merged 10 commits into from
May 15, 2025
Merged

Conversation

Azzedde
Copy link
Contributor

@Azzedde Azzedde commented May 5, 2025

Summary

This PR updates the documentation of the PartialDependenceDisplay class in sklearn/inspection/_plot/partial_dependence.py to mirror the style of other Display objects by linking both to the general Visualization API guide and to the interpretation section for partial dependence and ICE plots (issue #31304).

Changes

  • Changed the one-line summary to mention both PDP and ICE visualization.
  • Added a “For general information…” link to the :ref:\Visualization Guide ``.
  • Added a “For guidance on interpreting…” link to the :ref:\Partial Dependence and ICE plots section <partial_dependence>``.
  • Removed the old “Read more in … and the User Guide” phrasing and replaced with the dual-link format.

Motivation

Some Display classes in scikit-learn inconsistently point only to their low-level plotting function or only to the Visualizations guide. To give users both “how do I use this API?” and “what does this plot mean?” contexts, we standardize on dual linking—just like RocCurveDisplay.
Closes #31304.

How to test (if relevant)

  1. Build the HTML docs (make html) and open the Inspection → Partial Dependence page.

  2. Confirm that the PartialDependenceDisplay class doc now shows two guide links at the top:

    • “Visualization Guide” (general API)
    • “Partial Dependence and ICE plots section” (interpretation)
  3. Repeat for the from_estimator method doc.

  4. Ensure no rendering errors and that links resolve correctly.

Risks & considerations

  • Rendering: Verify the added RST references exist and render without broken links.
  • Backwards compatibility: Documentation only; no API behavior changes.
  • Consistency: Other Display docs will need similar updates (e.g. ConfusionMatrixDisplay, DetCurveDisplay, PrecisionRecallDisplay).

Additional information

This is one of several PRs addressing #31304. See related issues/PRs for metric Display classes.

Copy link

github-actions bot commented May 5, 2025

✔️ Linting Passed

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

Generated for commit: 475cab8. Link to the linter CI: here

@ArturoAmorQ
Copy link
Member

Thanks for the PR @Azzedde, can you please solve the conflicts?

@ArturoAmorQ ArturoAmorQ changed the title docs: updating docs in partial_dependence.py according to #31304 DOC Update docstring in partial_dependence.py May 12, 2025
@Azzedde
Copy link
Contributor Author

Azzedde commented May 12, 2025

@ArturoAmorQ the conflicts are solved now and all the checks passed 🫡
Expect me to contribute to more issues I really liked the process

Copy link
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @Azzedde. Here a batch of comments. Can you also please change the current line 42 from :ref:Partial Dependence and ICE plots <partial_dependence> to :ref:Inspection Guide <partial_dependence>?

sklearn/inspection/_plot/partial_dependence.py Outdated Show resolved Hide resolved
sklearn/inspection/_plot/partial_dependence.py Outdated Show resolved Hide resolved
sklearn/inspection/_plot/partial_dependence.py Outdated Show resolved Hide resolved
sklearn/inspection/_plot/partial_dependence.py Outdated Show resolved Hide resolved
sklearn/inspection/_plot/partial_dependence.py Outdated Show resolved Hide resolved
@Azzedde
Copy link
Contributor Author

Azzedde commented May 15, 2025

@ArturoAmorQ I did solve the comments you mentioned.

Thanks for the review !

Copy link
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

Thanks again for your work @Azzedde and congrats on your first merge!

@ArturoAmorQ ArturoAmorQ enabled auto-merge (squash) May 15, 2025 14:10
@Azzedde
Copy link
Contributor Author

Azzedde commented May 15, 2025

Thanks a lot,
I will contribute to other issues for sure !
🙏🙏🙏

@ArturoAmorQ ArturoAmorQ merged commit fc40a14 into scikit-learn:main May 15, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC Link Visualization tools to their respective interpretation in the User Guide
2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.