Skip to content

Commit

Permalink
Coord points and bounds inheritance. (SciTools#2553)
Browse files Browse the repository at this point in the history 
* Refactor Coord inheritance.

* Review changes.

* Review change -- reword comments for clarity.

* Fix Python3-specific problem.
  • Loading branch information
@pp-mo @bjlittle
pp-mo authored and bjlittle committed May 31, 2017
1 parent ed485a3 commit 09a0613
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 159 deletions.
299 changes: 149 additions & 150 deletions lib/iris/coords.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,13 +531,95 @@ def copy(self, points=None, bounds=None):

return new_coord

@abstractproperty
def points(self):
"""Property containing the points values as a numpy array"""
@classmethod
def from_coord(cls, coord):
"""Create a new Coord of this type, from the given coordinate."""
kwargs = {'points': coord.core_points(),
'bounds': coord.core_bounds(),
'standard_name': coord.standard_name,
'long_name': coord.long_name,
'var_name': coord.var_name,
'units': coord.units,
'attributes': coord.attributes,
'coord_system': copy.deepcopy(coord.coord_system)}
if issubclass(cls, DimCoord):
# DimCoord introduces an extra constructor keyword.
kwargs['circular'] = getattr(coord, 'circular', False)
return cls(**kwargs)

def _sanitise_array(self, src, ndmin):
if is_lazy_data(src):
# Lazy data : just ensure ndmin requirement.
ndims_missing = ndmin - src.ndim
if ndims_missing <= 0:
result = src
else:
extended_shape = tuple([1] * ndims_missing + list(src.shape))
result = src.reshape(extended_shape)
else:
# Real data : a few more things to do in this case.
# Ensure the array is writeable.
# NB. Returns the *same object* if src is already writeable.
result = np.require(src, requirements='W')
# Ensure the array has enough dimensions.
# NB. Returns the *same object* if result.ndim >= ndmin
result = np.array(result, ndmin=ndmin, copy=False)
# We don't need to copy the data, but we do need to have our
# own view so we can control the shape, etc.
result = result.view()
return result

def _points_getter(self):
"""The coordinate points values as a NumPy array."""
return self._points_dm.data

def _points_setter(self, points):
# Set the points to a new array - as long as it's the same shape.

# Ensure points has an ndmin of 1 and is either a numpy or lazy array.
# This will avoid Scalar coords with points of shape () rather
# than the desired (1,).
points = self._sanitise_array(points, 1)

# Set or update DataManager.
if self._points_dm is None:
self._points_dm = DataManager(points)
else:
self._points_dm.data = points

points = property(_points_getter, _points_setter)

def _bounds_getter(self):
"""
The coordinate bounds values, as a NumPy array,
or None if no bound values are defined.
.. note:: The shape of the bound array should be: ``points.shape +
(n_bounds, )``.
"""
bounds = None
if self.has_bounds():
bounds = self._bounds_dm.data
return bounds

def _bounds_setter(self, bounds):
# Ensure the bounds are a compatible shape.
if bounds is None:
self._bounds_dm = None
else:
bounds = self._sanitise_array(bounds, 2)
if self.shape != bounds.shape[:-1]:
raise ValueError("Bounds shape must be compatible with points "
"shape.")
if not self.has_bounds() \
or self.core_bounds().shape != bounds.shape:
# Construct a new bounds DataManager.
self._bounds_dm = DataManager(bounds)
else:
self._bounds_dm.data = bounds

@abstractproperty
def bounds(self):
"""Property containing the bound values as a numpy array"""
bounds = property(_bounds_getter, _bounds_setter)

def lazy_points(self):
"""
Expand Down Expand Up @@ -695,6 +777,15 @@ def _as_defn(self):
self.units, self.attributes, self.coord_system)
return defn

# Must supply __hash__ as Python 3 does not enable it if __eq__ is defined.
# NOTE: Violates "objects which compare equal must have the same hash".
# We ought to remove this, as equality of two coords can *change*, so they
# really should not be hashable.
# However, current code needs it, e.g. so we can put them in sets.
# Fixing it will require changing those uses. See #962 and #1772.
def __hash__(self):
return hash(id(self))

def __binary_operator__(self, other, mode_constant):
"""
Common code which is called by add, sub, mul and div
Expand Down Expand Up @@ -1440,16 +1531,6 @@ class DimCoord(Coord):
A coordinate that is 1D, numeric, and strictly monotonic.
"""
@staticmethod
def from_coord(coord):
"""Create a new DimCoord from the given coordinate."""
return DimCoord(coord.core_points(), standard_name=coord.standard_name,
long_name=coord.long_name, var_name=coord.var_name,
units=coord.units, bounds=coord.core_bounds(),
attributes=coord.attributes,
coord_system=copy.deepcopy(coord.coord_system),
circular=getattr(coord, 'circular', False))

@classmethod
def from_regular(cls, zeroth, step, count, standard_name=None,
long_name=None, var_name=None, units='1', attributes=None,
Expand Down Expand Up @@ -1547,10 +1628,13 @@ def __eq__(self, other):

# The __ne__ operator from Coord implements the not __eq__ method.

# This is necessary for merging, but probably shouldn't be used otherwise.
# See #962 and #1772.
def __hash__(self):
return hash(id(self))
# For Python 3, we must explicitly re-implement the '__hash__' method, as
# defining an '__eq__' has blocked its inheritance. See ...
# https://docs.python.org/3.1/reference/datamodel.html#object.__hash__
# "If a class that overrides __eq__() needs to retain the
# implementation of __hash__() from a parent class, the interpreter
# must be told this explicitly".
__hash__ = Coord.__hash__

def __getitem__(self, key):
coord = super(DimCoord, self).__getitem__(key)
Expand Down Expand Up @@ -1591,29 +1675,29 @@ def _new_points_requirements(self, points):
if len(points) > 1 and not iris.util.monotonic(points, strict=True):
raise ValueError('The points array must be strictly monotonic.')

@property
def points(self):
"""The local points values as a read-only NumPy array."""
return self._points_dm.data

@points.setter
def points(self, points):
# We must realise points to check monotonicity.
def _points_setter(self, points):
# DimCoord always realises the points, to allow monotonicity checks.
copy = is_lazy_data(points)
points = as_concrete_data(points)
points = np.array(points, copy=copy, ndmin=1)
# Ensure it is an actual array, and also make our own distinct view
# so that we can make it read-only.
points = np.array(points, copy=copy)

# Set or update DataManager.
if self._points_dm is None:
self._points_dm = DataManager(points)
else:
self._points_dm.data = points
# Invoke the generic points setter.
super(DimCoord, self)._points_setter(points)

if self._points_dm is not None:
# Re-fetch the core array, as the super call may replace it.
points = self._points_dm.core_data()
# N.B. always a *real* array, as we realised 'points' at the start.

# Check validity requirements for dimension-coordinate points.
self._new_points_requirements(points)

# Checks for 1d, numeric, monotonic
self._new_points_requirements(self._points_dm.data)
# Make the array read-only.
points.flags.writeable = False

# Make the array read-only.
self._points_dm.data.flags.writeable = False
points = property(Coord._points_getter, _points_setter)

def _new_bounds_requirements(self, bounds):
"""
Expand Down Expand Up @@ -1650,37 +1734,28 @@ def _new_bounds_requirements(self, bounds):
raise ValueError('The direction of monotonicity must be '
'consistent across all bounds')

@property
def bounds(self):
"""
The bounds values as a read-only NumPy array, or None if no
bounds have been set.
"""
bounds = None
if self.has_bounds():
bounds = self._bounds_dm.data
return bounds

@bounds.setter
def bounds(self, bounds):
if bounds is None:
self._bounds_dm = None
else:
def _bounds_setter(self, bounds):
if bounds is not None:
# Ensure we have a realised array of new bounds values.
copy = is_lazy_data(bounds)
bounds = as_concrete_data(bounds)
bounds = np.array(bounds, copy=copy, ndmin=2)
# Checks for appropriate values for the new bounds.
bounds = np.array(bounds, copy=copy)

# Invoke the generic bounds setter.
super(DimCoord, self)._bounds_setter(bounds)

if self._bounds_dm is not None:
# Re-fetch the core array, as the super call may replace it.
bounds = self._bounds_dm.core_data()
# N.B. always a *real* array, as we realised 'bounds' at the start.

# Check validity requirements for dimension-coordinate bounds.
self._new_bounds_requirements(bounds)

# Ensure the array is read-only.
bounds.flags.writeable = False
if not self.has_bounds() or self.bounds.shape != bounds.shape:
# Construct a new bounds DataManager.
self._bounds_dm = DataManager(bounds)
else:
self._bounds_dm.data = bounds

bounds = property(Coord._bounds_getter, _bounds_setter)

def is_monotonic(self):
return True
Expand All @@ -1694,97 +1769,21 @@ def xml_element(self, doc):


class AuxCoord(Coord):
"""A CF auxiliary coordinate."""
@staticmethod
def from_coord(coord):
"""Create a new AuxCoord from the given coordinate."""
new_coord = AuxCoord(coord.points, standard_name=coord.standard_name,
long_name=coord.long_name,
var_name=coord.var_name,
units=coord.units, bounds=coord.bounds,
attributes=coord.attributes,
coord_system=copy.deepcopy(coord.coord_system))

return new_coord

def _sanitise_array(self, src, ndmin):
if is_lazy_data(src):
# Lazy data : just ensure ndmin requirement.
ndims_missing = ndmin - src.ndim
if ndims_missing <= 0:
result = src
else:
extended_shape = tuple([1] * ndims_missing + list(src.shape))
result = src.reshape(extended_shape)
else:
# Real data : a few more things to do in this case.
# Ensure the array is writeable.
# NB. Returns the *same object* if src is already writeable.
result = np.require(src, requirements='W')
# Ensure the array has enough dimensions.
# NB. Returns the *same object* if result.ndim >= ndmin
result = np.array(result, ndmin=ndmin, copy=False)
# We don't need to copy the data, but we do need to have our
# own view so we can control the shape, etc.
result = result.view()
return result

@property
def points(self):
"""The local points values as a read-only NumPy array."""
return self._points_dm.data

@points.setter
def points(self, points):
# Set the points to a new array - as long as it's the same shape.

# Ensure points has an ndmin of 1 and is either a numpy or lazy array.
# This will avoid Scalar coords with points of shape () rather
# than the desired (1,).
points = self._sanitise_array(points, 1)

# Set or update DataManager.
if self._points_dm is None:
self._points_dm = DataManager(points)
else:
self._points_dm.data = points
"""
A CF auxiliary coordinate.
@property
def bounds(self):
"""
Property containing the bound values, as a NumPy array,
or None if no bound values are defined.
.. note::
.. note:: The shape of the bound array should be: ``points.shape +
(n_bounds, )``.
There are currently no specific properties of :class:`AuxCoord`,
everything is inherited from :class:`Coord`.
"""
bounds = None
if self.has_bounds():
bounds = self._bounds_dm.data
return bounds

@bounds.setter
def bounds(self, bounds):
# Ensure the bounds are a compatible shape.
if bounds is None:
self._bounds_dm = None
else:
bounds = self._sanitise_array(bounds, 2)
if self.shape != bounds.shape[:-1]:
raise ValueError("Bounds shape must be compatible with points "
"shape.")
if not self.has_bounds() \
or self.core_bounds().shape != bounds.shape:
# Construct a new bounds DataManager.
self._bounds_dm = DataManager(bounds)
else:
self._bounds_dm.data = bounds

# This is necessary for merging, but probably shouldn't be used otherwise.
# See #962 and #1772.
def __hash__(self):
return hash(id(self))
"""
# Logically, :class:`Coord` is an abstract class and all actual coords must
# be members of some concrete subclass, i.e. an :class:`AuxCoord` or
# a :class:`DimCoord`.
# So we retain :class:`AuxCoord` as a distinct concrete subclass.
# This provides clarity, backwards compatibility, and so we can add
# AuxCoord-specific code if needed in future.


class CellMeasure(six.with_metaclass(ABCMeta, CFVariableMixin)):
Expand Down
9 changes: 6 additions & 3 deletions lib/iris/cube.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3081,9 +3081,12 @@ def __ne__(self, other):
result = not result
return result

# Must supply __hash__, Python 3 does not enable it if __eq__ is defined
# This is necessary for merging, but probably shouldn't be used otherwise.
# See #962 and #1772.
# Must supply __hash__ as Python 3 does not enable it if __eq__ is defined.
# NOTE: Violates "objects which compare equal must have the same hash".
# We ought to remove this, as equality of two cube can *change*, so they
# really should not be hashable.
# However, current code needs it, e.g. so we can put them in sets.
# Fixing it will require changing those uses. See #962 and #1772.
def __hash__(self):
return hash(id(self))

Expand Down
5 changes: 3 additions & 2 deletions lib/iris/tests/test_coord_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,11 @@ def test_dim_coord_restrictions(self):
'monotonicity.*consistent.*all bounds'):
iris.coords.DimCoord([1, 2, 3], bounds=[[1, 12], [2, 9], [3, 6]])
# shapes of points and bounds
with self.assertRaisesRegexp(ValueError, 'shape of the bounds array'):
msg = 'Bounds shape must be compatible with points shape'
with self.assertRaisesRegexp(ValueError, msg):
iris.coords.DimCoord([1, 2, 3], bounds=[0.5, 1.5, 2.5, 3.5])
# another example of shapes of points and bounds
with self.assertRaisesRegexp(ValueError, 'shape of the bounds array'):
with self.assertRaisesRegexp(ValueError, msg):
iris.coords.DimCoord([1, 2, 3], bounds=[[0.5, 1.5], [1.5, 2.5]])

# numeric
Expand Down
Loading

0 comments on commit 09a0613

Please sign in to comment.