Minimal requirements #61
Conversation
| need: | ||
|
|
||
| * `matplotlib <http://matplotlib.org/>`_ | ||
| * `healpy <https://healpy.readthedocs.io/en/latest/>`_ |
There was a problem hiding this comment.
healpy is only required for one type of plot, so its doubly optional.
There was a problem hiding this comment.
I think it might be two plots, or at least healpy appears in two functions.
There was a problem hiding this comment.
Indeed, healpy is required by both plot_healpix_map() and plot_sky_binned().
There was a problem hiding this comment.
Yes, you're right. But healpix-deniers can still make pretty plots without installing healpy if they insist.
py/desiutil/plots.py
Outdated
| numpy.ma.MaskedArray.__array_finalize__(self, obj) | ||
| self.vmin = getattr(obj, 'vmin', None) | ||
| self.vmax = getattr(obj, 'vmax', None) | ||
| # def __array_finalize__(self, obj): |
There was a problem hiding this comment.
I guess you checked this dunder method isn't actually needed? (I just copied the example)
There was a problem hiding this comment.
I'm not sure what a "dunder" method is, but I do know that the original code was not working as expected, and the new code is. Please see the unit tests.
There was a problem hiding this comment.
Ok, thanks. (dunder ~ "double underscore").
| if mask.shape != data.shape: | ||
| # | ||
| # Make every effort to ensure that modifying the mask of the output | ||
| # does not modify the input mask. |
There was a problem hiding this comment.
How does renaming mask to cmask accomplish this?
There was a problem hiding this comment.
It's not a renaming, it's a copy. That function was modifying its inputs in unexpected ways. In the original code, if you input a masked array, and then input the same masked array into the function again, the inputs will not be the same.
There was a problem hiding this comment.
Note: np.asarray(x) will not make a copy if x is already an array, while np.array(x) will make a copy. Both will promote lists to arrays if needed (thus giving more input flexibility than x.copy()). i.e. I think the code below could have been implemented as simply cmask = np.array(mask) without needing try/except.
There was a problem hiding this comment.
Indeed, the simplified cmask = np.array(mask) appears to work, and is simpler.
|
Will merge as soon as the last round of tests pass. |
This PR: