Addressing issue #589: Adding alpha params to PCA #807
Conversation
|
@naresh-bachwani This PR still has changes for residuals. |
|
@naresh-bachwani you have changes for both residuals and PCA in this PR. This needs to be rectified before we move forward |
|
Hello @lwgray, |
|
@naresh-bachwani I am inclined to say to wait until merging #806. Then once your develop is up-to-date, you should create a feature-branch off of develop for the PCA alpha work. |
|
If you like, I can still make comments here while we wait then you can apply them in the new PR |
|
OK. That will be great |
| @@ -164,7 +164,7 @@ def test_biplot_3d(self): | |||
| pca_array = visualizer.transform(self.dataset.X) | |||
|
|
|||
| # Image comparison tests | |||
| self.assert_images_similar(visualizer) | |||
| self.assert_images_similar(visualizer,tol=10) | |||
|
|
|||
| # Assert PCA transformation occurred successfully | |||
| assert pca_array.shape == (self.dataset.X.shape[0], 3) | |||
There was a problem hiding this comment.
We might want to add a test for alpha. It could be similar to the alpha test found in residuals.py
There was a problem hiding this comment.
OK. I will construct a function for alpha test.
| @@ -118,7 +119,7 @@ def __init__(self, | |||
| [('scale', StandardScaler(with_std=self.scale)), | |||
| ('pca', PCA(self.proj_dim, random_state=random_state))] | |||
| ) | |||
|
|
|||
| self.alpha=alpha | |||
There was a problem hiding this comment.
This is exactly right. Additionally, We should include alpha parameter in the docstring above
| @@ -154,7 +155,7 @@ def transform(self, X, y=None, **kwargs): | |||
| def draw(self, **kwargs): | |||
| X = self.pca_features_ | |||
| if self.proj_dim == 2: | |||
| self.ax.scatter(X[:, 0], X[:, 1], c=self.color, cmap=self.colormap) | |||
| self.ax.scatter(X[:, 0], X[:, 1], c=self.color, cmap=self.colormap,alpha=self.alpha) | |||
There was a problem hiding this comment.
place a space after the comma right before alpha
There was a problem hiding this comment.
OK. Will do so!
| @@ -177,7 +178,7 @@ def draw(self, **kwargs): | |||
| self.fig = plt.figure() | |||
| self.ax = self.fig.add_subplot(111, projection='3d') | |||
| self.ax.scatter(X[:, 0], X[:, 1], X[:, 2], | |||
| c=self.color, cmap=self.colormap) | |||
| c=self.color, cmap=self.colormap,alpha=self.alpha) | |||
There was a problem hiding this comment.
place a space after the comma right before alpha
There was a problem hiding this comment.
OK. Will do so!
| @@ -215,7 +216,7 @@ def finalize(self, **kwargs): | |||
|
|
|||
| def pca_decomposition(X, y=None, ax=None, features=None, scale=True, | |||
| proj_dim=2, proj_features=False, color=None, | |||
| colormap=palettes.DEFAULT_SEQUENCE, | |||
| colormap=palettes.DEFAULT_SEQUENCE,alpha=1, | |||
There was a problem hiding this comment.
place a space after the comma right before alpha
There was a problem hiding this comment.
OK. Will do so!
|
@naresh-bachwani #806 was merged so whenever you are ready to open a new PR for PCA alpha, go ahead. Just make sure you local develop branch is in sync with upstream yellowbrick develop branch prior to creating any new feature branches |
|
@lwgray, I will make a new PR once I have my test for alpha ready!! |
|
Hello, @lwgray @rebeccabilbro, |
|
@naresh-bachwani let’s move this discussion to the new PR even if not all the tests are passing. |
|
Ok! I am closing this PR and opening a new one. |
The following changes were made to PCA referenced #589 :
pca.pyto specify the opacity.test_pca.pyThe result is attached below:
