Addresses Issue #615 : Adding new parameters "Colorbar" and "Heatmap" in PCA visualizer

[percygautam]

The following changes were made to pca.py referenced in #615 :

• Added new optional parameters 'Colorbar' and 'Heatmap' which shows the magnitude of each feature value to the component, for both '2D' and '3D' cases.

Next Step:

• Updating Documentation with a proper explanation on how to use these two optional parameters when using PCA visualizer.

P.S: I am adding a snippet showing PCA visualization of the dataset credit from yellowbrick.datasets, when both the parameters are True and proj_dim = 2.

[percygautam]

@rebeccabilbro Hey, there.
I have corrected the transform method, which now works with ax object passed. I am getting difficulties in getting the axis for heatmap to set x and y labels in the finalize method. Currently, I am using plt.gca() method to get the axis. How can I fix this issue?

[naresh-bachwani]

Hey @percygautam,
I will be working on PCA this summer, so I thought to have a look at your solution. You have done a great job! To answer your query can you try using something like self.ax1.set_xticks()?( where ax1 is the axis of the heatmap) Let me know if it works. However, wait till @rebeccabilbro reviews it. She may have something to add.

[rebeccabilbro]

Hello there @percygautam and so glad to see this moving forward!

My recommendation is definitely along the lines of what @naresh-bachwani is suggesting (P.S. thanks for jumping in here @naresh-bachwani and very glad you're keeping track of this so that you'll be ready to jump in with the other PCA roadmap improvements once this is merged! Potentially you and I should team up in reviewing this PR once @percygautam is ready for us!).

@percygautam I propose that you read through our latest improvements to JointPlotVisualizer, take a look at what we do to divide up the axes (depending on which params the user passes in), and come up with a similar solution. In JointPlot we have a two axes that are None by default (though probably for PCA you'd call them something like self.uax and self.lax for "upper axes" and "lower axes"). We also have a private method that divides the axes up iff certain parameters have been set by the user (e.g. colorbar and heatmap in the case of PCA). You can call this method on init, after which you will be able to reference self.uax and self.lax in finalize to add whatever finishing touches are needed (e.g. self.uax.set_xticks() like @naresh-bachwani suggested).

Hope this helps and keep us posted!

[percygautam]

Hey @rebeccabilbro @naresh-bachwani
Thanks, for your suggestions. I have used the methods used for JointPlot and now the heatmap and colorbar are rendering properly for 2d projections. But this method will not work for 3d projections as we can't get the axis of 3d scatter plot. Currently, I am using subplot for rendering 3d plot, but this method is not feasible! Any suggestions for 3d projections?

[rebeccabilbro]

Thanks, for your suggestions. I have used the methods used for JointPlot and now the heatmap and colorbar are rendering properly for 2d projections. But this method will not work for 3d projections as we can't get the axis of 3d scatter plot. Currently, I am using subplot for rendering 3d plot, but this method is not feasible! Any suggestions for 3d projections?

I propose adding logic so that the multi-axes are only available if the user selects a 2D plot (e.g. raise an error if proj_dim == 3, and either heatmap == True or colorbar == True).

Also, @percygautam — it looks like for some reason your PR has integrated commits from an unrelated PR for the new Cook's Distance visualizer. Would you please remove all of those additions from this PR so that we will not have a merge conflict?

[percygautam]

@rebeccabilbro I have done the changes you mentioned.

[rebeccabilbro]

Ok, great @percygautam! I've just pulled from develop to make sure your branch is synced up (make sure you also do a pull from this feature branch into your local copy before you push any additional changes); once the automated tests have finished running I'll start my review!

[naresh-bachwani]

Hey there @percygautam,
Sorry for the delay in reply. As I mentioned earlier, you have done a great job. I have added a few comments on your approach. Feel free to ask any questions.
Apart from this, it would be great if you could update the docstring with the new parameters.
Testing is an essential feature of YB library. It makes sure that everything is working as it is supposed to. I would love to work with you to add tests for the new features. If you're open to it, we can further discuss these.

[percygautam]

@naresh-bachwani Thank you for your suggestions. I have done the changes you mentioned.

[rebeccabilbro]

Hi again @percygautam and shout-out to @naresh-bachwani for doing the first review of this! I noticed there was a bit of a merge conflict because of a PR that we just merged in that affected a lot of the test files, so I went ahead and resolved the conflict, updated the images, and pushed them to your branch.

This PR looks like it's 95% of the way there! I think there are only a few things left to do (@naresh-bachwani let me know if I missed anything else!):

• address some of the test failures (this may take some experimenting with some of the text-removal tools for our visual comparison tests)
• update the parameters listed in the docstring for the PCA visualizer class to include colorbar and heatmap and explain how are meant to be used and what their default values are
• update the parameters listed in the docstring for quickmethod to include colorbar and heatmap and explain how are meant to be used and what their default values are
• add docstrings to explain what the draw method and finalize method do

@percygautam let me know if you'd like me to help wrap things up with the image comparison tests, etc — I'd be happy to help!

[naresh-bachwani]

Hey there @rebeccabilbro and @percygautam,
Thank you @percygautam for making the changes. The tests are looking good. Just a few minor changes and we will have the tests passing. As @rebeccabilbro said, these tests are failing (probably) due to text differences. So, we just need to remove the text. To achieve that you can remove the finalize calls from the test. Or you can also explicitly set ticks to None.

@rebeccabilbro, we added the following tests:

1. Test heatmap
2. Test colorbar
3. Test heatmap and colorbar
4. Test the error when the projection dimension is 3 and heatmap or colorbar is true.

Are there any other tests that you think we must be adding? Apart from this and docstring, all other things are looking great!

[bbengfort]

@rebeccabilbro @naresh-bachwani -- it looks like the text removal tools in the image similarity tests are only affecting the primary Axes - and the text that is causing the issues here are on the two new axes of the colorbar and the heatmap. I've opened up an issue #916 that will address this.

@percygautam - For now, would you mind removing the ticks explicitly in the tests as @naresh-bachwani suggested? Please put a comment with a # TODO: manually modifying ticks should be removed after #916 is fixed in the places that you make that modification.

[rebeccabilbro]

@percygautam — just checking in, how's your progress on this? We're so close!

[percygautam]

@rebeccabilbro Sorry for the delay in progress! Thank you, @naresh-bachwani and @bbengfort for your inputs. I'll do the changes mentioned by today!

[percygautam]

@rebeccabilbro I have done some changes! I am having some problem with pytest, will change the baseline images once it's resolved.

[percygautam]

@rebeccabilbro @naresh-bachwani After removing text from the images, tests are still failing. How to rectify this?

[rebeccabilbro]

Great work @percygautam! I've pushed up a few small additions (e.g. adding in docstrings for some of the methods that didn't have them so that they'll show up nicely in our Sphinx docs) & the tests are now running again. As soon as everything passes, I think we'll be ready to merge this in — thanks for sticking this out to the end! 😃