Improve the performance of CompositeMap plotting #6085
Conversation
| @@ -11,6 +11,8 @@ | |||
| import sunpy.map | |||
| from sunpy.tests.helpers import figure_test | |||
|
|
|||
| import test_plotting | |||
There was a problem hiding this comment.
I don't think you need this import.
| if wcsaxes_compat.gca_wcs(m.wcs)==axes.wcs: | ||
| params['autoalign'] = False | ||
| elif wcsaxes_compat.is_wcsaxes(axes): | ||
| params['autoalign'] = True |
There was a problem hiding this comment.
How does this speed up the plotting? I am not familiar with CompositeMap plotting
There was a problem hiding this comment.
See #5894. We get improved performance if we set autoalign=False when the map layer is being plotted onto axes with a matching WCS because then we use the faster imshow() instead of the slower pcolormesh().
There was a problem hiding this comment.
Ah, so the idea is to bypass using pcolormesh when it makes sense?
There was a problem hiding this comment.
Yup, precisely, because it's glacial in an interactive plot.
| @@ -523,7 +527,7 @@ def peek(self, colorbar=True, draw_limb=True, draw_grid=False, **matplot_args): | |||
| raise TypeError("draw_grid should be bool, int, long or float") | |||
|
|
|||
| return figure | |||
|
|
|||
There was a problem hiding this comment.
I marked this PR as "draft" since it is not finished, here's stuff that needs to be fixed in it:
- You access
axes.wcsbefore you check ifaxesis aWCSAxesinstance. - Calling
gca_wcs()doesn't make sense because (1) it will return the currentpyplotaxes if one exists, and thus may not correspond tom.wcsat all, and (2) if a user hasn't been usingpyplot, it will create a new figure and axes throughpyplot. - You (attempt to) import
test_plotting.py, but I don't know why. - Your unit test appears to be adapted from
test_plot_autoalign()fromtest_plotting.py, but I suspect you did not run your unit test at all because your code tries to access attributes and methods that aren't available forCompositeMap. Plus, you copied over the routine, but it would work as intended only if it is also marked as a figure test. - Even if your unit test did actually run, it doesn't test the functionality that this PR is trying to add because you are trying to plot the composite map onto a non-matching WCS. That does and should continue to use
autoalign=True. A proper test would likely not be a figure test – because the figure output would be very similar and possibly even equal – but instead would probably have to check whether amatplotlib.collections.QuadMeshwas attached to the plot when a single composite-map layer is plotted onto a matching WCS.
nabobalis
changed the title
|
Hi @ooprathamm, thanks for the PR. Hopefully you will have time in the future to update this PR using the comments I and ayshih left! I wanted to quickly explain what I use draft status for, its for any PR that is not passing the majority of checks and probably means it isn't ready for a "formal" review. This does not mean that no one will look at it, but normally a non-draft PR is in a position to be merged into the repository 90% of the time. |
|
Thank you for the PR @ooprathamm, if you have a chance to work on this in figure, please do reopen this PR or a new one! |
Partial attempt at fixing #5894