Better default bounds for Image

drs251 commented 7 years ago

Currently, the default behavior for Image is to apply bounds of +-0.5 to both axes. If the original data is not square shaped, the result is a distorted image, as shown below:

%%opts Image (cmap="gray") [aspect='equal']
import holoviews as hv
from scipy import misc
face = misc.face(gray=True)
hv.Image(face)

The original image was 1024x768, but is now shown squashed. When no bounds are specified, the bounds should be set so that the data's original aspect ratio is preserved, e.g. something like this: 1) Set bounds on x axis to -0.5 and +0.5 2) Set bounds on y axis to -a and +a, where a=data.shape[1]/data.shape[0]*0.5

ea42gh commented 7 years ago

I would argue for setting bounds to matrix row and column indices. This would also alert the user to the layout conventions for the vertical axis

drs251 commented 7 years ago

Or it might be nice to replicate the behaviour of Raster as the default setting, to make it easier to switch between the two. Anyway, as long as the aspect of the original data is preserved, I'd be happy.

jlstevens commented 7 years ago

I think reproducing some of Rasters behavior by default is definitely a possibility we should consider. If Image can cover the use-case of Raster well, it might be nicer to deprecate Raster entirely instead of having to write a data interface to support it (something we would have to do otherwise).

drs251 commented 7 years ago

One thing to keep in mind is that in Raster the vertical axis goes from top to bottom, whereas it's the other way around for Image and RGB (I think some of the others as well?), without the possibility to change it. It would be nice to be consistent across Holoviews, so deprecateing Raster would help - and it would make choosing the correct class for 2D data a little bit easier.

michaelaye commented 7 years ago

First, thanks for the awesome package!

For me, Raster behaves like an Image displayer should behave:

Respecting the aspect of the data
Starting the y-axis count from top to bottom, as is customary for any image sensor readouts.

However, there's obviously need for 2D raster displays that have both zero coords in the lower left, together, like all mathematically derived 2D data arrays, similar to pcolor/pcolormesh in matplotlib.

So I'd like to see 2 classes for 2D grid displays: one for focusing on image sensor data that respects the incoming aspect for sure, and a second one that not necessarily does that, and might be set by default to a square aspect. So, naming-wise I would say that the current status is inverted to the best case scenario, because Raster is doing what I would expect from an Image object display and v.v.

jlstevens commented 7 years ago

It wouldn't be too hard to get Image to respect the aspect by default (at least if a bounds isn't explicitly set). The issue then is one regarding coordinate systems which I don't think should be configurable per element.

One thing I might suggest is that Raster is a subclass of Image which fixes the bounds to be in sample space (so it will work with integer indexing) with the appropriate coordinate system (i.e not unit bounds and without the origin at the center).

That said, I'm still not sure what the best approach is but I do know I would like to have things improved by 2.0.

jlstevens commented 7 years ago

I would like to summarize a plan I discussed with Philipp:

Make Raster support the gridded interface, so it uses discrete indexing that can either be computed automatically (given a simple 2D array) but that can also include other discrete indexing such as datetimes. Even sampling will be assumed but not enforced. Supporting the grid interface will make it easier to use whatever conventions you want.
Flip the y-axis of image so that arr[0,0] is displayed as the bottom left pixel not the top-left one. Then we can keep the current Cartesian axes which is consistent with how charts are displayed i.e 'x' increasing from left to right, 'y' increasing moving upwards. From our understanding, this convention would be more intuitive for many users.
For compatibility we can add a config flag to restore the current behavior and we can add FlippedImage in 2.0 which flips the y-axis which will behave the way Image does now.
It turns out supporting aspects can be made to work for matplotlib by setting appropriate bounds and specifying a square aspect but anything involving aspects is much trickier to handle in bokeh. For this reason, we aren't quite sure yet about how to handle aspects.

@philippjfr Is that a fair summary? Maybe we should still set the bounds from the data shape so aspects at least work with matplotlib?

michaelaye commented 7 years ago

Do I understand that you propose to not have any class that flips an image array with (0,0) in the top left? I would bet that all image/remote sensing ppl are rather more familiar with that layout, and in fact most space imagery defaults to this. So I vote for having at least one class doing this by default, just as plt.imshow does.

jewfro-cuban commented 5 years ago

[Edited]

So first of all.. great package. I'm making my first steps here so some help would be appreciated.

I've created a first version of ImageTL, a subclass of Image that by default plots the image with the (0,0) at the top-left corner. In addition, by default, the aspect ratio of the rendered array is preserved.

Please review and suggest how to finalize it (merge it with Image ?) https://github.com/jewfro-cuban/holoviews

Test nb here: https://github.com/jewfro-cuban/holoviews/blob/master/holoviews/tests/ipython/notebooks/test_image_tl.ipynb

Usage example:

import skimage
import numpy as np
import holoviews as hv
hv.extension('bokeh')

data = skimage.data.stereo_motorcycle()[0][:, :, 0]   # 2d np.ndarray
hv.ImageTL(data)

produces:

holoviz / holoviews

Better default bounds for Image #1534