Skip to content

Commit

Permalink
Notimplemented binop (#5073)
Browse files Browse the repository at this point in the history
* layout: Fix __add__ and __radd__ implementation

* Add layout.Layoutable as a mirror of overlay.Overlayable
* Remove a good deal of duplicated code
* Remove broken calls to super().__radd__ where the super class does not
  implement __radd__
* Return NotImplemented when Layout([x,y]) raises NotImplementedError.
  This allows correct interoperability with external classes that could
  themselves define __radd__ as stated by:
  https://docs.python.org/3/library/constants.html#NotImplemented

Fixes #3577

* overlay: deduplicate and fix __mul__

* Return NotImplemented when appropriate
* Deduplicate code between 2 non-trivial and almost identical
  implementations of __mul__
* Fix non-inheritance-friendly type checking with a local import to
  avoid cyclic dependency

Fixes #3577
  • Loading branch information
douglas-raillard-arm authored and philippjfr committed Nov 16, 2021
1 parent 82577ba commit fbd7a29
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 99 deletions.
75 changes: 26 additions & 49 deletions holoviews/core/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,33 @@
from . import traversal


class Composable(object):
class Layoutable:
"""
Composable is a mix-in class to allow Dimensioned objects to be
embedded within Layouts and GridSpaces.
Layoutable provides a mix-in class to support the
add operation for creating a layout from the operands.
"""

def __add__(self, obj):
def __add__(x, y):
"Compose objects into a Layout"
return Layout([self, obj])

def __radd__(self, other):
if isinstance(other, int):
raise TypeError("unsupported operand type(s) for +: 'int' and 'Overlay'. "
if any(isinstance(arg, int) for arg in (x, y)):
raise TypeError(f"unsupported operand type(s) for +: {x.__class__.__name__} and {y.__class__.__name__}. "
"If you are trying to use a reduction like `sum(elements)` "
"to combine a list of elements, we recommend you use "
"`Layout(elements)` (and similarly `Overlay(elements)` for "
"making an overlay from a list) instead.")
try:
return Layout([x, y])
except NotImplementedError:
return NotImplemented

def __radd__(self, other):
return self.__class__.__add__(other, self)


class Composable(Layoutable):
"""
Composable is a mix-in class to allow Dimensioned objects to be
embedded within Layouts and GridSpaces.
"""

def __lshift__(self, other):
"Compose objects into an AdjointLayout"
Expand Down Expand Up @@ -57,7 +67,7 @@ def __init__(self):



class AdjointLayout(Dimensioned):
class AdjointLayout(Layoutable, Dimensioned):
"""
An AdjointLayout provides a convenient container to lay out some
marginal plots next to a primary plot. This is often useful to
Expand Down Expand Up @@ -311,27 +321,13 @@ def __iter__(self):
yield self[i]
i += 1


def __add__(self, obj):
"Composes plot into a Layout with another object."
return Layout([self, obj])

def __radd__(self, other):
if isinstance(other, int):
raise TypeError("unsupported operand type(s) for +: 'int' and 'Overlay'. "
"If you are trying to use a reduction like `sum(elements)` "
"to combine a list of elements, we recommend you use "
"`Layout(elements)` (and similarly `Overlay(elements)` for "
"making an overlay from a list) instead.")
return super(AdjointLayout, self).__radd__(self, other)

def __len__(self):
"Number of items in the AdjointLayout"
return len(self.data)



class NdLayout(UniformNdMapping):
class NdLayout(Layoutable, UniformNdMapping):
"""
NdLayout is a UniformNdMapping providing an n-dimensional
data structure to display the contained Elements and containers
Expand Down Expand Up @@ -388,21 +384,6 @@ def cols(self, ncols):
self._max_cols = ncols
return self


def __add__(self, obj):
"Composes the NdLayout with another object into a Layout"
return Layout([self, obj])


def __radd__(self, other):
if isinstance(other, int):
raise TypeError("unsupported operand type(s) for +: 'int' and 'Overlay'. "
"If you are trying to use a reduction like `sum(elements)` "
"to combine a list of elements, we recommend you use "
"`Layout(elements)` (and similarly `Overlay(elements)` for "
"making an overlay from a list) instead.")
return super(NdLayout, self).__radd__(self, other)

@property
def last(self):
"""
Expand Down Expand Up @@ -440,7 +421,7 @@ def clone(self, *args, **overrides):
return clone


class Layout(ViewableTree):
class Layout(Layoutable, ViewableTree):
"""
A Layout is an ViewableTree with ViewableElement objects as leaf
values. Unlike ViewableTree, a Layout supports a rich display,
Expand Down Expand Up @@ -561,16 +542,12 @@ def grid_items(self):
return {tuple(np.unravel_index(idx, self.shape)): (path, item)
for idx, (path, item) in enumerate(self.items())}


def __add__(self, other):
"Composes the Layout with another object returning a merged Layout."
return Layout([self, other])

def __mul__(self, other, reverse=False):
from .spaces import HoloMap
if not isinstance(other, (ViewableElement, HoloMap)):
if isinstance(other, (ViewableElement, HoloMap)):
return Layout([other*v if reverse else v*other for v in self])
else:
return NotImplemented
return Layout([other*v if reverse else v*other for v in self])

def __rmul__(self, other):
return self.__mul__(other, reverse=True)
Expand Down
5 changes: 4 additions & 1 deletion holoviews/core/ndmapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,10 @@ def __mul__(self, other, reverse=False):

overlayed_items = [(k, other * el if reverse else el * other)
for k, el in self.items()]
return self.clone(overlayed_items)
try:
return self.clone(overlayed_items)
except NotImplementedError:
return NotImplemented


def __rmul__(self, other):
Expand Down
55 changes: 21 additions & 34 deletions holoviews/core/overlay.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,21 @@
import param
from .dimension import Dimension, Dimensioned, ViewableElement, ViewableTree
from .ndmapping import UniformNdMapping
from .layout import Composable, Layout, AdjointLayout
from .layout import Composable, Layout, AdjointLayout, Layoutable
from .util import sanitize_identifier, unique_array, dimensioned_streams


class Overlayable(object):
class Overlayable:
"""
Overlayable provides a mix-in class to support the
mul operation for overlaying multiple elements.
"""

def __mul__(self, other):
"Overlay object with other object."
if type(other).__name__ == 'DynamicMap':
# Local import to break the import cyclic dependency
from .spaces import DynamicMap

if isinstance(other, DynamicMap):
from .spaces import Callable
def dynamic_mul(*args, **kwargs):
element = other[args]
Expand All @@ -34,14 +36,21 @@ def dynamic_mul(*args, **kwargs):
callback._is_overlay = True
return other.clone(shared_data=False, callback=callback,
streams=dimensioned_streams(other))
if isinstance(other, UniformNdMapping) and not isinstance(other, CompositeOverlay):
items = [(k, self * v) for (k, v) in other.items()]
return other.clone(items)
elif isinstance(other, (AdjointLayout, ViewableTree)) and not isinstance(other, Overlay):
return NotImplemented

return Overlay([self, other])
else:
if isinstance(self, Overlay):
if not isinstance(other, ViewableElement):
return NotImplemented
else:
if isinstance(other, UniformNdMapping) and not isinstance(other, CompositeOverlay):
items = [(k, self * v) for (k, v) in other.items()]
return other.clone(items)
elif isinstance(other, (AdjointLayout, ViewableTree)) and not isinstance(other, Overlay):
return NotImplemented

try:
return Overlay([self, other])
except NotImplementedError:
return NotImplemented


class CompositeOverlay(ViewableElement, Composable):
Expand Down Expand Up @@ -124,7 +133,7 @@ def dimension_values(self, dimension, expanded=True, flat=True):
return vals if expanded else unique_array(vals)


class Overlay(ViewableTree, CompositeOverlay):
class Overlay(ViewableTree, CompositeOverlay, Layoutable, Overlayable):
"""
An Overlay consists of multiple Elements (potentially of
heterogeneous type) presented one on top each other with a
Expand Down Expand Up @@ -171,28 +180,6 @@ def get(self, identifier, default=None):
return default
return super(Overlay, self).get(identifier, default)


def __add__(self, other):
"Composes Overlay with other object into a Layout"
return Layout([self, other])


def __mul__(self, other):
"Adds layer(s) from other object to Overlay"
if type(other).__name__ == 'DynamicMap':
from .spaces import Callable
def dynamic_mul(*args, **kwargs):
element = other[args]
return self * element
callback = Callable(dynamic_mul, inputs=[self, other])
callback._is_overlay = True
return other.clone(shared_data=False, callback=callback,
streams=dimensioned_streams(other))
elif not isinstance(other, ViewableElement):
return NotImplemented
return Overlay([self, other])


def collate(self):
"""
Collates any objects in the Overlay resolving any issues
Expand Down
18 changes: 3 additions & 15 deletions holoviews/core/spaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
from . import traversal, util
from .accessors import Opts, Redim
from .dimension import OrderedDict, Dimension, ViewableElement
from .layout import Layout, AdjointLayout, NdLayout, Empty
from .layout import Layout, AdjointLayout, NdLayout, Empty, Layoutable
from .ndmapping import UniformNdMapping, NdMapping, item_check
from .overlay import Overlay, CompositeOverlay, NdOverlay, Overlayable
from .options import Store, StoreOptions
from ..streams import Stream, Params, streams_list_from_dict



class HoloMap(UniformNdMapping, Overlayable):
class HoloMap(Layoutable, UniformNdMapping, Overlayable):
"""
A HoloMap is an n-dimensional mapping of viewable elements or
overlays. Each item in a HoloMap has an tuple key defining the
Expand Down Expand Up @@ -305,12 +305,6 @@ def dynamic_mul(*args, **kwargs):
else:
return NotImplemented


def __add__(self, obj):
"Composes HoloMap with other object into a Layout"
return Layout([self, obj])


def __lshift__(self, other):
"Adjoin another object to this one returning an AdjointLayout"
if isinstance(other, (ViewableElement, UniformNdMapping, Empty)):
Expand Down Expand Up @@ -1856,7 +1850,7 @@ def next(self):



class GridSpace(UniformNdMapping):
class GridSpace(Layoutable, UniformNdMapping):
"""
Grids are distinct from Layouts as they ensure all contained
elements to be of the same type. Unlike Layouts, which have
Expand Down Expand Up @@ -1969,12 +1963,6 @@ def __len__(self):
"""
return max([(len(v) if hasattr(v, '__len__') else 1) for v in self.values()] + [0])


def __add__(self, obj):
"Composes the GridSpace with another object into a Layout."
return Layout([self, obj])


@property
def shape(self):
"Returns the 2D shape of the GridSpace as (rows, cols)."
Expand Down

0 comments on commit fbd7a29

Please sign in to comment.