Skip to content
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

Grid based interface fixes #794

Merged
merged 26 commits into from
Jul 27, 2016
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
5428392
Ensure DictInterface does not consume unexpected types
philippjfr Jul 18, 2016
a280914
Reverted fix to flipped iris interface
philippjfr Jul 18, 2016
4426274
Generalized and fixed xarray constructor
philippjfr Jul 18, 2016
4124f29
Fix for XArrayInterface values method
philippjfr Jul 18, 2016
0079735
Pass parameters through in dynamic groupby
philippjfr Jul 18, 2016
f8e4ede
Fixed GridImage dimension declarations
philippjfr Jul 18, 2016
33b316a
Fix for DictInterface constructor
philippjfr Jul 18, 2016
5fd7ef4
Fixed CubeInterface test
philippjfr Jul 18, 2016
b80437c
Fixed dynamic groupby parameter passing
philippjfr Jul 18, 2016
a984d64
Added inverted coordinate systems in grid interfaces
philippjfr Jul 25, 2016
46bb8fa
Fixed GridImage density
philippjfr Jul 25, 2016
98725de
Added unit tests for Dataset
philippjfr Jul 25, 2016
5d08919
Avoid duplication of invert method on grid interfaces
philippjfr Jul 25, 2016
75031a5
Further fixes for grid interface orientations
philippjfr Jul 25, 2016
9004db4
Small fix for reorienting 1D arrays
philippjfr Jul 25, 2016
b29cd21
HeatMap avoids reaggregating grid based data
philippjfr Jul 25, 2016
30196f1
Revert bad fix for xarray 1D dimension_values
philippjfr Jul 25, 2016
cb461b1
Further orientation fixes for grid interfaces
philippjfr Jul 25, 2016
4ecb993
GridInterface reindex fixes
philippjfr Jul 25, 2016
2c8aeed
Refactored grid interface canonicalization code into one method
philippjfr Jul 26, 2016
3b5b042
Handle views into higher dimensional gridded datasets
philippjfr Jul 26, 2016
b557f84
Added attribute to interface to check for gridded data
philippjfr Jul 27, 2016
3dbdbd4
Added comment to clarify dict interface type handling
philippjfr Jul 27, 2016
6e8e46f
Fixed various error messages in interfaces
philippjfr Jul 27, 2016
309bedc
Renamed GridInterface get_coords to coords
philippjfr Jul 27, 2016
1ee04d7
Factored grid coordinate expansion out into utility
philippjfr Jul 27, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion holoviews/core/data/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,15 @@ def groupby(self, dimensions=[], container_type=HoloMap, group_type=None,

if dynamic:
group_dims = [d.name for d in self.kdims if d not in dimensions]
group_kwargs = dict(util.get_param_values(self), **kwargs)
group_kwargs['kdims'] = [self.get_dimension(d) for d in group_dims]
def load_subset(*args):
constraint = dict(zip(dim_names, args))
group = self.select(**constraint)
if np.isscalar(group):
return group_type(([group],), group=self.group,
label=self.label, vdims=self.vdims)
return group_type(group.reindex(group_dims))
return group_type(group.reindex(group_dims), **group_kwargs)
dynamic_dims = [d(values=list(self.interface.values(self, d.name, False)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks right, but if I am not confused this is a separate fix from the rest of the PR? Looks like the kwargs weren't being passed into the group container when dynamic....

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That caused weird bugs on the interfaces before I fixed the grid interfaces. So it's related at least.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to have the fix! I just wanted to make sure I understood it...

for d in dimensions]
return DynamicMap(load_subset, kdims=dynamic_dims)
Expand Down
3 changes: 2 additions & 1 deletion holoviews/core/data/dictionary.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def init(cls, eltype, data, kdims, vdims):
data = {k: data[:,i] for i,k in enumerate(dimensions)}
elif isinstance(data, list) and np.isscalar(data[0]):
data = {dimensions[0]: np.arange(len(data)), dimensions[1]: data}
elif not isinstance(data, dict):
elif not any(isinstance(data, tuple(t for t in interface.types if t is not None))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just add a comment to say this is needed as dictionary can incorrectly accept xarray (duck typing fails here!). The comment might also mention that a cleaner approach would be welcome...

for interface in cls.interfaces.values()):
data = {k: v for k, v in zip(dimensions, zip(*data))}
elif isinstance(data, dict) and not all(d in data for d in dimensions):
dict_data = zip(*((util.wrap_tuple(k)+util.wrap_tuple(v))
Expand Down
95 changes: 78 additions & 17 deletions holoviews/core/data/grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def init(cls, eltype, data, kdims, vdims):
if shape != expected[::-1] and not (not expected and shape == (1,)):
raise ValueError('Key dimension values and value array %s '
'shape do not match. Expected shape %s, '
'actual shape: %s' % (vdim, expected, shape))
'actual shape: %s' % (vdim, expected[::-1], shape))
return data, {'kdims':kdims, 'vdims':vdims}, {}


Expand Down Expand Up @@ -101,19 +101,80 @@ def length(cls, dataset):
return np.product([len(dataset.data[d.name]) for d in dataset.kdims])


@classmethod
def get_coords(cls, dataset, dim, ordered=False):
"""
Returns the coordinates along a dimension.
"""
data = dataset.data[dim]
if ordered and np.all(data[1:] < data[:-1]):
data = data[::-1]
return data


@classmethod
def expanded_coords(cls, dataset, dim):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be folded into get_coords above using an expanded=False default argument?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I've never been a fan of get_ as a prefix. How about simple coords or coordinates?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, just coords is fine.

arrays = [cls.get_coords(dataset, d.name, True)
for d in dataset.kdims]
idx = dataset.get_dimension_index(dim)
return util.cartesian_product(arrays)[idx]


@classmethod
def canonicalize(cls, dataset, data, coord_dims=None):
"""
Canonicalize takes an array of values as input and
reorients and transposes it to match the canonical
format expected by plotting functions. In addition
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably too much of a pain to stick in the docstring but it would be good to describe our canonical format properly somewhere in our docs. Not something that should hold up this PR though!

to the dataset and the particular array to apply
transforms to a list of coord_dims may be supplied
in case the array indexing does not match the key
dimensions of the dataset.
"""
if coord_dims is None:
coord_dims = dataset.dimensions('key', True)

# Reorient data
invert = False
slices = []
for d in coord_dims:
coords = cls.get_coords(dataset, d)
if np.all(coords[1:] < coords[:-1]):
slices.append(slice(None, None, -1))
invert = True
else:
slices.append(slice(None))
data = data.__getitem__(slices[::-1]) if invert else data

# Transpose data
dims = [name for name in coord_dims[::-1]
if isinstance(cls.get_coords(dataset, name), np.ndarray)]
dropped = [dims.index(d) for d in dims if d not in dataset.kdims]
inds = [dims.index(kd.name) for kd in dataset.kdims]
inds += dropped
if inds:
data = data.transpose(inds[::-1])

# Allow lower dimensional views into data
if len(dataset.kdims) < 2:
data = data.flatten()
elif dropped:
data = data.squeeze(axis=tuple(range(len(dropped))))
return data


@classmethod
def values(cls, dataset, dim, expanded=True, flat=True):
if dim in dataset.kdims:
if not expanded:
return dataset.data[dim]
prod = util.cartesian_product([dataset.data[d.name] for d in dataset.kdims])
idx = dataset.get_dimension_index(dim)
values = prod[idx]
return values.flatten() if flat else values
else:
if dim in dataset.vdims:
dim = dataset.get_dimension(dim)
values = dataset.data.get(dim.name)
return values.T.flatten() if flat else values
data = dataset.data.get(dim.name)
data = cls.canonicalize(dataset, data)
return data.T.flatten() if flat else data
elif expanded:
data = cls.expanded_coords(dataset, dim)
return data.flatten() if flat else data
else:
return cls.get_coords(dataset, dim, True)


@classmethod
Expand Down Expand Up @@ -282,17 +343,17 @@ def reindex(cls, dataset, kdims, vdims):
if k not in dropped_kdims+dropped_vdims}

if kdims != dataset.kdims:
dropped_axes = tuple(dataset.ndims-dataset.kdims.index(d)-1
joined_dims = kdims+dropped_kdims
axes = tuple(dataset.ndims-dataset.kdims.index(d)-1
for d in joined_dims)
dropped_axes = tuple(dataset.ndims-joined_dims.index(d)-1
for d in dropped_kdims)
old_kdims = [d for d in dataset.kdims if not d in dropped_kdims]
axes = tuple(dataset.ndims-old_kdims.index(d)-1
for d in kdims)
for vdim in vdims:
vdata = data[vdim.name]
if len(axes) > 1:
vdata = vdata.transpose(axes[::-1])
if dropped_axes:
vdata = vdata.squeeze(axis=dropped_axes)
if len(axes) > 1:
vdata = np.transpose(vdata, axes)
data[vdim.name] = vdata
return data

Expand Down
30 changes: 15 additions & 15 deletions holoviews/core/data/iris.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,30 +118,30 @@ def validate(cls, dataset):
pass


@classmethod
def get_coords(cls, dataset, dim, ordered=False):
data = dataset.data.coords(dim)[0].points
if ordered and np.all(data[1:] < data[:-1]):
data = data[::-1]
return data


@classmethod
def values(cls, dataset, dim, expanded=True, flat=True):
"""
Returns an array of the values along the supplied dimension.
"""
dim = dataset.get_dimension(dim)
if dim in dataset.vdims:
data = dataset.data.copy().data
coord_names = [c.name() for c in dataset.data.dim_coords
if c.name() in dataset.kdims]
if flat:
dim_inds = [coord_names.index(d.name) for d in dataset.kdims]
dim_inds += [i for i in range(len(dataset.data.dim_coords))
if i not in dim_inds]
data = data.transpose(dim_inds)
else:
data = np.flipud(data)
coord_names = [c.name() for c in dataset.data.dim_coords]
data = dataset.data.copy().data.T
data = cls.canonicalize(dataset, data, coord_names)
return data.T.flatten() if flat else data
elif expanded:
idx = dataset.get_dimension_index(dim)
data = util.cartesian_product([dataset.data.coords(d.name)[0].points
for d in dataset.kdims])[idx]
data = cls.expanded_coords(dataset, dim)
return data.flatten() if flat else data
else:
data = dataset.data.coords(dim.name)[0].points
return data.flatten() if flat else data
return cls.get_coords(dataset, dim.name, True)


@classmethod
Expand Down
63 changes: 36 additions & 27 deletions holoviews/core/data/xarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,37 +40,40 @@ def init(cls, eltype, data, kdims, vdims):
vdim_param = element_params['vdims']

if kdims:
kdim_names = [kd.name if isinstance(kd, Dimension) else kd for kd in kdims]
kdim_names = [kd.name if isinstance(kd, Dimension)
else kd for kd in kdims]
else:
kdim_names = [kd.name for kd in eltype.kdims]

if not isinstance(data, xr.Dataset):
ndims = len(kdim_names)
kdims = [kd if isinstance(kd, Dimension) else Dimension(kd)
for kd in kdims]
vdim = vdims[0].name if isinstance(vdims[0], Dimension) else vdims[0]
vdims = [vd if isinstance(vd, Dimension) else Dimension(vd)
for vd in vdims]
if isinstance(data, tuple):
value_array = np.array(data[-1])
data = {d: vals for d, vals in zip(kdim_names + [vdim], data)}
elif isinstance(data, dict):
value_array = np.array(data[vdim])
if value_array.ndim > 1:
value_array = value_array.T
dims, coords = zip(*[(kd.name, data[kd.name])
for kd in kdims])
data = {d.name: vals for d, vals in zip(kdims + vdims, data)}
if not isinstance(data, dict):
raise TypeError('XArrayInterface could not interpret data type')
coords = [(kd.name, data[kd.name]) for kd in kdims][::-1]
arrays = {}
for vdim in vdims:
arr = data[vdim.name]
if not isinstance(arr, xr.DataArray):
arr = xr.DataArray(arr, coords=coords)
arrays[vdim.name] = arr
try:
arr = xr.DataArray(value_array, coords=coords, dims=dims)
data = xr.Dataset({vdim: arr})
data = xr.Dataset(arrays)
except:
pass
if not isinstance(data, xr.Dataset):
raise TypeError('Data must be be an xarray Dataset type.')

if isinstance(data, xr.Dataset):
else:
if vdims is None:
vdims = list(data.data_vars.keys())
if kdims is None:
kdims = list(data.dims.keys())
kdims = [name for name in data.dims
if isinstance(data[name].data, np.ndarray)]

if not isinstance(data, xr.Dataset):
raise TypeError('Data must be be an xarray Dataset type.')
return data, {'kdims': kdims, 'vdims': vdims}, {}


Expand Down Expand Up @@ -116,20 +119,26 @@ def groupby(cls, dataset, dimensions, container_type, group_type, **kwargs):
return container_type(data)


@classmethod
def get_coords(cls, dataset, dim, ordered=False):
data = dataset.data[dim].data
if ordered and np.all(data[1:] < data[:-1]):
data = data[::-1]
return data


@classmethod
def values(cls, dataset, dim, expanded=True, flat=True):
data = dataset.data[dim].data
if dim in dataset.vdims:
if data.ndim == 1:
return np.array(data)
else:
return data.T.flatten() if flat else data
elif not expanded:
return data
coord_dims = dataset.data[dim].dims[::-1]
data = cls.canonicalize(dataset, data, coord_dims=coord_dims)
return data.T.flatten() if flat else data
elif expanded:
data = cls.expanded_coords(dataset, dim)
return data.flatten() if flat else data
else:
arrays = [dataset.data[d.name].data for d in dataset.kdims]
product = util.cartesian_product(arrays)[dataset.get_dimension_index(dim)]
return product.flatten() if flat else product
return cls.get_coords(dataset, dim, True)


@classmethod
Expand Down
14 changes: 9 additions & 5 deletions holoviews/element/raster.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
import param

from ..core import util
from ..core.data import (ArrayInterface, NdElementInterface, DictInterface)
from ..core.data import (ArrayInterface, NdElementInterface,
DictInterface, GridInterface)
from ..core import (Dimension, NdMapping, Element2D,
Overlay, Element, Dataset, NdElement)
from ..core.boundingregion import BoundingRegion, BoundingBox
Expand Down Expand Up @@ -396,6 +397,8 @@ def __init__(self, data, extents=None, **params):


def _compute_raster(self):
if issubclass(self.interface, GridInterface):
return self, np.flipud(self.dimension_values(2, flat=False))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps all interfaces could have a property to check to see if this return can be done directly instead of using isinstance?

d1keys = self.dimension_values(0, False)
d2keys = self.dimension_values(1, False)
coords = [(d1, d2, np.NaN) for d1 in d1keys for d2 in d2keys]
Expand Down Expand Up @@ -648,16 +651,17 @@ class GridImage(Dataset, Element2D):

group = param.String(default='GridImage', constant=True)

kdims = param.List(default=['x', 'y'], bounds=(2, 2))
kdims = param.List(default=[Dimension('x'), Dimension('y')],
bounds=(2, 2))

vdims = param.List(default=['z'], bounds=(1, 1))
vdims = param.List(default=[Dimension('z')], bounds=(1, 1))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought strings were automatically promoted to Dimension objects and that we favored strings by default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not as a default.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at various examples in element I see you are right. Now I'm wondering how it worked at all before when using the default!


def __init__(self, data, **params):
super(GridImage, self).__init__(data, **params)
(l, r), (b, t) = self.interface.range(self, 0), self.interface.range(self, 1)
(ys, xs) = self.dimension_values(2, flat=False).shape
xsampling = (float(r-l)/xs)/2.
ysampling = (float(t-b)/ys)/2.
xsampling = (float(r-l)/(xs-1))/2.
ysampling = (float(t-b)/(ys-1))/2.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an off-by-one error in the bounds? How come this wasn't flagged up by the tests...or did you update the test data? If not, why isn't bounds being checked in the data comparisons?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was on GridImage, and is currently untested.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, that makes sense.

l, r = l-xsampling, r+xsampling
b, t = b-ysampling, t+ysampling
self.bounds = BoundingBox(points=((l, b), (r, t)))
Expand Down
2 changes: 1 addition & 1 deletion holoviews/plotting/bokeh/raster.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def _update_glyph(self, glyph, properties, mapping):
class ImagePlot(RasterPlot):

def get_data(self, element, ranges=None, empty=False):
img = np.flipud(element.dimension_values(2, flat=False))
img = element.dimension_values(2, flat=False)
l, b, r, t = element.bounds.lbrt()
dh, dw = t-b, r-l
mapping = dict(image='image', x='x', y='y', dw='dw', dh='dh')
Expand Down
2 changes: 1 addition & 1 deletion holoviews/plotting/mpl/raster.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def update_handles(self, key, axis, element, ranges, style):
class ImagePlot(RasterPlot):

def get_data(self, element, ranges, style):
data = element.dimension_values(2, flat=False)
data = np.flipud(element.dimension_values(2, flat=False))
Copy link
Contributor

@jlstevens jlstevens Jul 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come this doesn't flip Image given dimension_values for numpy arrays won't be based on interface classes?

Edit: I now realize this isn't RasterPlot and is only handling GridImage so ignore this comment!

data = np.ma.array(data, mask=np.logical_not(np.isfinite(data)))
vdim = element.vdims[0]
self._norm_kwargs(element, ranges, style, vdim)
Expand Down
Loading