Add column / row wise normalization to imshow and pyrshow#49
Add column / row wise normalization to imshow and pyrshow#49ershook wants to merge 1 commit intoLabForComputationalVision:mainfrom
Conversation
billbrod
left a comment
billbrod
left a comment
There was a problem hiding this comment.
Thanks for putting this together Erica! I don't have time to do a full review right now, so I've added some small comments and will get back to this in more detail tomorrow.
| def _get_clims(fig): | ||
| """get vmin vmax for each image in fig as list of tuples (vmin, vmax)""" | ||
| return [ax.images[0].get_clim() for ax in fig.axes if ax.images] | ||
|
|
||
| # define test images such that each image has a different range of values, | ||
| # so we can test that the correct vrange is applied to each one | ||
| IMAGES = [np.arange(4 * i, 4 * i + 4, dtype=float).reshape(2, 2) for i in range(4)] |
There was a problem hiding this comment.
These are only used by TestVrange, right? they should be moved within that class
| with self.subTest(mode=mode, img_idx=img_idx): | ||
| fig = self._imshow(f"auto{mode}") | ||
| clim_vmin, clim_vmax = _get_clims(fig)[img_idx] | ||
| title_vmin, title_vmax = _get_title_clims(fig)[img_idx] |
There was a problem hiding this comment.
the function _get_title_clims is missing, so all tests fail when I run this locally. can you add it?
| If a 2-tuple, specifies the image values vmin/vmax that are mapped to | ||
| (ie. clipped to) the minimum and maximum value of the colormap, | ||
| respectively. | ||
| If a list of 2-tuples, the length of number of images, each image has an |
There was a problem hiding this comment.
the indent here is wrong to make the docs render correctly (and some of that is our fault predating your PR, but we may as well fix it all now). Can you reformat this (and the corresponding docstrings in imshow, pyrshow, animshow) look like this? (you can also see it in the corresponding docstring from plenoptic)
|
|
||
|
|
||
| def colormap_range(image, vrange='indep1', cmap=None): | ||
| def colormap_range(image, vrange= 'indep1', cmap=None, n_rows = None, n_cols = None): |
There was a problem hiding this comment.
add sections to Arguments documenting n_rows, n_cols
also, and this is a pre-existing issue, can you add one for cmap? something like:
cmap : matplotlib colormap, optional
colormap to use when showing these images. If None, will pick RdBu_r if vrange is some variant of auto0 or indep0, else will pick gray.
2 participants
This PR: