DM-20551: Add a number of bug-fixes and requested features to display_matplotlib #11
Conversation
There was a problem hiding this comment.
Some minor things that need fixing. I'm also not sure what to do about the single commit. It doesn't follow the normal approach of splitting commits into standalone pieces but at minimum can the first line be fixed so that it doesn't look like the commit is solely about implementing erase?
| if where in orientationDict: | ||
| orientation = orientationDict[where] | ||
| else: | ||
| print(f"Unknown location {where}; " + |
There was a problem hiding this comment.
Please remove the + at the end since I don't think it's needed.
examples/display_matplotlib.ipynb
Outdated
| "im = afwImage.MaskedImageF(100, 50)\n", | ||
| "afwMath.randomGaussianImage(im.image, afwMath.Random())\n", | ||
| "\n", | ||
| "sim = im[afwGeom.BoxI(afwGeom.PointI(0, 0), afwGeom.ExtentI(im.getWidth()//2, im.getHeight()))]\n", |
There was a problem hiding this comment.
Please use geom and not afwGeom here (also needs fixing earlier)
There was a problem hiding this comment.
I did that; but missed the examples. Done
| ra = apAngle(ra*u.deg).to_string(unit=u.hour, **kwargs) | ||
| dec = apAngle(dec*u.deg).to_string(unit=u.deg, **kwargs) | ||
| else: | ||
| ra = ("%9.4f") % ra |
There was a problem hiding this comment.
I don't think the parens are doing anything here. You can alternatively do:
ra = f"{ra:9.4f}"| for a in ax.get_images(): | ||
| a.get_cursor_data = lambda ev: None # disabled | ||
|
|
||
| if False: |
There was a problem hiding this comment.
If we are keeping this block in but disabled may be explain in what context we should use True here?
There was a problem hiding this comment.
I changed the if False block to a comment on why I didn't use tight_layout()
| @@ -600,6 +734,9 @@ def _pan(self, colc, rowc): | |||
| def _getEvent(self, timeout=-1): | |||
| """Listen for a key press, returning (key, x, y)""" | |||
|
|
|||
| if timeout < 0: | |||
| timeout = 24*3600 # -1 generates complaints in QTimer::singleShot | |||
There was a problem hiding this comment.
Do we want to comment why 1 day is a good timeout number?
There was a problem hiding this comment.
It just needs to be a long time; I added that
|
|
||
| \param fig The figure to monitor for keyboard events | ||
| @param fig The figure to monitor for keyboard events |
There was a problem hiding this comment.
Maybe switch to numpydoc like some of the changes earlier?
There was a problem hiding this comment.
This is an orthogonal change that I'd rather do when I'm not rushing to get this into a weekly
| vmin, vmax = self._getMinMaxQ()[0:2] | ||
| if vmax*Q > vmin: | ||
| vmax *= Q | ||
| Normalize.__init__(self, vmin, vmax) |
There was a problem hiding this comment.
Python2 code. I'll make all the super changes requested
|
|
||
| # The object used to perform the desired mapping | ||
| self.mapping = afwRgb.AsinhZScaleMapping(image, Q) | ||
|
|
||
| vmin, vmax = self._getMinMaxQ()[0:2] | ||
| Normalize.__init__(self, vmin, vmax) |
There was a problem hiding this comment.
Add a comment if you are deliberately bypassing the AsinhNormalize.__init__ to explain why super() is not being used.
| # The object used to perform the desired mapping | ||
| self.mapping = afwRgb.ZScaleMapping(image, nSamples, contrast) | ||
|
|
||
| Normalize.__init__(self, self.mapping.minimum[0], self.mapping.maximum) |
| # The object used to perform the desired mapping | ||
| self.mapping = afwRgb.LinearMapping(minimum, maximum) | ||
|
|
||
| Normalize.__init__(self, self.mapping.minimum[0], self.mapping.maximum) |
Implement erase() properly Support sexagesimal formatting of coordinates Either via a parameter to the ctor, or via an extension API; both called useSesagesimal Also make dpi a real arg, not kwargs (why did I do that?) Fix matplotlib errors/warnings Show WCS in degrees not radians Set better vmax for asinh stretches Add support for passing a matplotlib figure as the frame set_colorbar() the default Obey scale(..., maskedPixel=XXX) Currently only implemented for minmax; see DM-16565
RobertLuptonTheGood
force-pushed
the
tickets/DM-20551
branch
from
c797200
to
f779797
Compare
Support sexagesimal formatting of coordinates
Either via a parameter to the ctor, or via an extension API;
both called useSesagesimal
Also make dpi a real arg, not kwargs (why did I do that?)
Fix matplotlib errors/warnings
Show WCS in degrees not radians
Set better vmax for asinh stretches
Add support for passing a matplotlib figure as the frame
set_colorbar() the default
Obey scale(..., maskedPixel=XXX)
Currently only implemented for minmax; see DM-16565