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
Use pcolormesh for image plots involving non-uniform axes #3192
Conversation
Indeed case A and B are the most important ones. Case B is quite common, as the navigator of a linescan could be such a plot (currently it is a lineplot of the integrated intensity). So I would put priority on these two cases for the moment. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## RELEASE_next_major #3192 +/- ##
======================================================
- Coverage 81.25% 81.24% -0.01%
======================================================
Files 173 173
Lines 24192 24202 +10
Branches 5621 5624 +3
======================================================
+ Hits 19657 19663 +6
- Misses 3236 3238 +2
- Partials 1299 1301 +2
☔ View full report in Codecov by Sentry. |
In principle that would be still OK, but there will be no more minor release before version 2.0 envisaged for september. So if this PR can make it into v2.0, it will get into a major release. |
The tests on azure pipelines are failing ... I guess some images are now different than tested for? |
I tested the implementation on some real data (linescans, thus the |
A point I noticed, also when testing hyperspy/rosettasciio#87, is that for non-uniform axes, the aspect ratio of image plots in not really ideal (x-axis is much shorter than y-axis). It is not possible to correct this using the Also, for navigators as well as for |
Yes, |
I already experimented with the aspect ratio, also due to hyperspy/rosettasciio#87. Here are my takeaways: For uniform images, the axis scale is used to set the default aspect ratio. Thus images with axes having the same size always result in square images (independent of scale/offset). If the axes sizes are too different, For non-uniform axes, the aspect ratio is always set to 1 independent of Furthermore, there is to add that for |
Found the problem, didn't run pytest with --mpl. |
@pietsjoh it might be also good to add a test which makes sure that the tests are plotting correctly using the --mpl compare. If you haven't already looked https://hyperspy.org/hyperspy-doc/dev/dev_guide/testing.html#plot-testing this is a good reference for how to add those tests. Just a note that those tests require specific versions of matplotlib and a couple of other packages to work properly.
|
I have updated the existing non-uniform axes plot tests. The tests cover both implemented cases (1d sig, 1d nav; 2d sig). However, I would prefer using structured data instead of random data. I think this would be better to detect any problems when the test fails. So should I add a new test or modify the existing ones? Furthermore, a test is needed to check whether the update works as intended when changing the navigation coordinates. Here I am unsure how to do this exactly. To be more specific: How do I change the navigation coordinates for plot (behaviour of "Ctrl+ Arrow" for example) in a python script? |
After a suggestion from @jlaehne I used |
LGTM, anyone else want to have a look @CSSFrancis @ericpre ? |
Is there any reason to use |
Good question, I have always used It seems to be hard though to find a comparison of the two methods. Though matplotlib/matplotlib#18652 (comment) suggests that Though with pcolormesh, we might have to be aware that "x,y values that represent the corners of the mesh, not the center points": matplotlib/matplotlib#18652 (comment) |
If you think that |
I didn't know about |
Well, I didn't know neither, but I didn't expect to see that it is necessary to use a collections, so I started to look it up in matplotlib and came across |
So I looked a little deeper into if self.yaxis.is_uniform and self.xaxis.is_uniform:
...
else:
new_args_non_uniform = dict()
new_args_non_uniform.update(new_args)
new_args_non_uniform.pop("vmin", None)
new_args_non_uniform.pop("vmax", None)
new_args_non_uniform.pop("aspect", None)
im = matplotlib.image.NonUniformImage(self.ax, **new_args_non_uniform)
im.set_data(self.xaxis.axis, self.yaxis.axis, data)
self.ax.add_image(im)
self.ax.set_xlim(self._extent[0], self._extent[1])
self.ax.set_ylim(self._extent[2], self._extent[3]) The following image showcases the mentioned difference in representation for a simple plot. The following code reproduces the figure. import numpy as np
import matplotlib.pyplot as plt
from matplotlib.image import NonUniformImage
x = np.linspace(-4, 4, 3)
y = np.linspace(-4, 4, 3)
z = np.sqrt(x[np.newaxis, :]**2 + y[:, np.newaxis]**2)
fig, axs = plt.subplots(ncols=2, nrows=1)
ax = axs[0]
im = NonUniformImage(ax, extent=(-4, 4, -4, 4))
im.set_data(x, y, z)
ax.add_image(im)
ax.set_xlim(-4, 4)
ax.set_ylim(-4, 4)
ax.set_title("NonUniformImage")
axs[1].pcolormesh(x, y, z)
axs[1].set_title("pcolormesh")
plt.show() Let's have a look at the extra arguments. In hyperspy these are tailored for Finally, I want to give a comprehensive list of the pros and cons for using
I am happy to implement either one. Just let me know which one you prefer. |
Thanks @pietsjoh for this detailed comparison! In the end, I would still tend to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @pietsjoh, indeed, it seems that pcolormesh
is more sensible, particularly for the vmin
/vmax
.
A quick search on github (during which I came across matplotlib/matplotlib#7763) also showed that it is clearly more widely used than NonUniformImage
.
Description of the change
As mentioned in #2398 it would be better to use
pcolormesh
instead ofimshow
for image plots with non-uniform axes.This switch doesn't affect image plots with only uniform axes. It only gets used when atleast 1 of the 2 axes of the image is non-uniform.
As far as I can tell, there are essentially 3 different ways to get an image plot:
As for (A): implementing this is not a problem, I just need to check if the update step works as intended.
Plotting (B) and (C) also involves widgets. The line widgets seem to also work with
pcolormesh
and thus (B) also works. However, case (C) is more tricky. TheSquareWidget
updates the position correctly, but not the size of the widget. Switching toRectangularWidget
looks difficult, because it heavily relies on the scale attribute of the axis.I will check how to update the size, but I am not sure if this functionality is necessary. Because most non-uniform axes are signal axes anyways, hence supporting
pcolormesh
for 2D nav plots might not be widely used.Progress of the PR
upcoming_changes
folder (seeupcoming_changes/README.rst
),readthedocs
doc build of this PR (link in github checks)Minimal example of the bug fix or the new feature
Plot generated by the code above.
top=current(imshow)
bottom=change(pcolormesh)
Notes
I based this on
RELEASE_next_minor
, but I am unsure if this correct. This PR does not affect hyperspy's api directly. However,pcolormesh
accepts different args thanimshow
. For examples.plot(aspect=100)
passesaspect
toimshow
. Butpcolormesh
doesn't have this arg and will throw an error. Thus backwards compatibility might be broken for some scripts.