8000 Added colorblind compatibility for decomposition example by johannah · Pull Request #5574 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

Added colorblind compatibility for decomposition example #5574

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

Conversation

johannah
Copy link
Contributor

Colorblind compatibility as discussed in #5435.

plot_incremental_pca.py
figure_1
figure_2

plot_pca_vs_lda.py
figure_1

figure_2

plot_sparse_coding.py
figure_1

@arjoly
Copy link
Member
arjoly commented Oct 23, 2015

LGTM !

@johannah johannah force-pushed the module-decomposition-colorblind branch from 8639f80 to b980117 Compare October 23, 2015 17:19
@jmschrei
Copy link
Member

LGTM as well, assuming all checks pass.

@@ -40,11 +40,14 @@
pca = PCA(n_components=n_components)
X_pca = pca.fit_transform(X)

colors = ['navy', 'turquoise', 'darkorange']
lw = 2
Copy link
Member

Choose a reason for hiding this comment

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

It's a somewhat minor comment, but I would rather avoir adding a variable for something that is used only once. In other words, I would prefer if this value was just inlined in the call to "scatter".

@johannah johannah force-pushed the module-decomposition-colorblind branch from b980117 to 0c29002 Compare October 26, 2015 19:47
@ogrisel
Copy link
Member
ogrisel commented Oct 28, 2015

The circle ci failure is unrelated (network failure). Merging.

ogrisel added a commit that referenced this pull request Oct 28, 2015
Added colorblind compatibility for decomposition example
@ogrisel ogrisel merged commit 6c56b25 into scikit-learn:master Oct 28, 2015
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.

5 participants
0