8000 DOC: Remove non-relevant comment in `fetch_lfw_pairs` documentation by star1327p · Pull Request #30871 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC: Remove non-relevant comment in fetch_lfw_pairs documentation #30871

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 5 commits into from
Mar 4, 2025

Conversation

star1327p
Copy link
Contributor
@star1327p star1327p commented Feb 21, 2025

Reference Issues/PRs

Related to #30800.

What does this implement/fix? Explain your changes.

Remove a comment in fetch_lfw_pairs documentation about leaving the "unrestricted' variant unsupported.
https://scikit-learn.org/dev/modules/generated/sklearn.datasets.fetch_lfw_pairs.html

The README.txt link does not work, so we rather mention the original paper.

Any other comments?

Copy link
github-actions bot commented Feb 21, 2025

✔️ Linting Passed

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

Generated for commit: 71682e6. Link to the linter CI: here

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.

I think the information that the "pairs dataset" corresponds to the "restricted task" in the original publication is actually valuable. I would edit to something similar to:

    In the `original paper <https://people.cs.umass.edu/~elm/papers/lfw.pdf>`_
    the "pairs" version corresponds to the "restricted task", where
    the experimenter should not use the name of a person to infer
    the equivalence or non-equivalence of two face images that
    are not explicitly given in the training set.

And then I agree that we should remove the comment on not supporting the "unrestricted version", which actually corresponds to the "people" version in my understanding of things.

@star1327p
Copy link
Contributor Author

@ArturoAmorQ Thank you for your feedback! I have added the narrative per your suggestion.

@ArturoAmorQ ArturoAmorQ changed the title DOC: Remove an extra paragraph in fetch_lfw_pairs documentation DOC: Remove non-relevant comment in fetch_lfw_pairs documentation Mar 4, 2025
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 @star1327p! Merging!

@ArturoAmorQ ArturoAmorQ merged commit d0ee195 into scikit-learn:main Mar 4, 2025
35 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.

2 participants
0