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

DOC Reorganize plot_nca_illustration example into subsections #14795

Merged
merged 4 commits into from Sep 18, 2019
Merged

DOC Reorganize plot_nca_illustration example into subsections #14795

merged 4 commits into from Sep 18, 2019

Conversation

m-clare
Copy link
Contributor

@m-clare m-clare commented Aug 24, 2019

Reference Issues/PRs

Partial fixes #14703

What does this implement/fix? Explain your changes.

Reorganize examples/neighbors/plot_nca_illustration.py example into subsections

Any other comments?

Working with @olgadk7 at WiMLDS sprint

Copy link
Member

@NicolasHug NicolasHug 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 @m-clare and @olgadk7.

Made a few comments but mostly looks good

examples/neighbors/plot_nca_illustration.py Outdated Show resolved Hide resolved
examples/neighbors/plot_nca_illustration.py Show resolved Hide resolved
sklearn/kernel_approximation.py Outdated Show resolved Hide resolved
plt.show()

##############################################################################
# We use :class:`~sklearn.neighbors.NeighborhoodComponentsAnalysis` to learn an
Copy link
Member

Choose a reason for hiding this comment

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

Add a title e.g. "learning the embedding"?

@NicolasHug
Copy link
Member

Can you please address the comments @m-clare ?

@reshamas
Copy link
Member

reshamas commented Sep 9, 2019

@kellycarmody,
Can you drop Olga and M an email? Thanks.

@NicolasHug
Copy link
Member

@reshamas just FYI, I made sure all PRs were reviewed by Monday after the sprint (2 weeks ago.)

I pinged the authors on Monday last week, and this morning I addressed the comments myself when the authors didn't answer.

@reshamas
Copy link
Member

reshamas commented Sep 9, 2019

@NicolasHug
ok, thanks for the update. If there are any outstanding PRs, @ingrid88 is also looking to work on additional issues, so she can assist. Ingrid attended the sprint.

@NicolasHug
Copy link
Member

Quick look @thomasjpfan ? This is from the sprint

@thomasjpfan thomasjpfan changed the title [MRG] Reorganize plot_nca_illustration example into subsections DOC Reorganize plot_nca_illustration example into subsections Sep 18, 2019
@thomasjpfan thomasjpfan merged commit 6680bff into scikit-learn:master Sep 18, 2019
@NicolasHug
Copy link
Member

Thanks @m-clare @olgadk7 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reorganize examples into different subsections
4 participants