From 5f31faaa5a3d74351a6d46807dbb315967fd401f Mon Sep 17 00:00:00 2001 From: Ken Kroenlein Date: Wed, 10 Jan 2024 13:52:32 -0700 Subject: [PATCH 1/2] Move real/integer bounds attributes to properties --- gemd/entity/bounds/integer_bounds.py | 57 ++++++++++++++-------- gemd/entity/bounds/real_bounds.py | 50 ++++++++++++++----- setup.py | 2 +- tests/entity/bounds/test_integer_bounds.py | 9 +++- tests/entity/bounds/test_real_bounds.py | 14 ++++-- 5 files changed, 92 insertions(+), 40 deletions(-) diff --git a/gemd/entity/bounds/integer_bounds.py b/gemd/entity/bounds/integer_bounds.py index 7c8f0ef3..8dbce68d 100644 --- a/gemd/entity/bounds/integer_bounds.py +++ b/gemd/entity/bounds/integer_bounds.py @@ -1,34 +1,51 @@ """Bounds an integer to be between two values.""" +from math import isfinite +from typing import Union + from gemd.entity.bounds.base_bounds import BaseBounds -from typing import Union +__all__ = ["IntegerBounds"] class IntegerBounds(BaseBounds, typ="integer_bounds"): - """ - Bounded subset of the integers, parameterized by a lower and upper bound. + """Bounded subset of the integers, parameterized by a lower and upper bound.""" - Parameters - ---------- - lower_bound: int - Lower endpoint. - upper_bound: int - Upper endpoint. + def __init__(self, lower_bound, upper_bound): + self._lower_bound = None + self._upper_bound = None - """ - - def __init__(self, lower_bound=None, upper_bound=None): self.lower_bound = lower_bound self.upper_bound = upper_bound - if self.lower_bound is None or abs(self.lower_bound) >= float("inf"): - raise ValueError("Lower bound must be given and finite: {}".format(self.lower_bound)) - - if self.upper_bound is None or abs(self.upper_bound) >= float("inf"): - raise ValueError("Upper bound must be given and finite") - - if self.upper_bound < self.lower_bound: - raise ValueError("Upper bound must be greater than or equal to lower bound") + @property + def lower_bound(self) -> int: + """The lower endpoint of the permitted range.""" + return self._lower_bound + + @lower_bound.setter + def lower_bound(self, value: int): + """Set the lower endpoint of the permitted range.""" + if value is None or not isfinite(value): + raise ValueError(f"Lower bound must be given and finite: {value}") + if self.upper_bound is not None and value > self.upper_bound: # Set first + raise ValueError(f"Upper bound ({self.upper_bound}) must be " + f"greater than or equal to lower bound ({value})") + self._lower_bound = int(value) + + @property + def upper_bound(self) -> int: + """The upper endpoint of the permitted range.""" + return self._upper_bound + + @upper_bound.setter + def upper_bound(self, value: int): + """Set the upper endpoint of the permitted range.""" + if value is None or not isfinite(value): + raise ValueError(f"Upper bound must be given and finite: {value}") + if value < self.lower_bound: + raise ValueError(f"Upper bound ({value}) must be " + f"greater than or equal to lower bound ({self.lower_bound})") + self._upper_bound = int(value) def contains(self, bounds: Union[BaseBounds, "BaseValue"]) -> bool: # noqa: F821 """ diff --git a/gemd/entity/bounds/real_bounds.py b/gemd/entity/bounds/real_bounds.py index c49c9455..cc888975 100644 --- a/gemd/entity/bounds/real_bounds.py +++ b/gemd/entity/bounds/real_bounds.py @@ -1,9 +1,10 @@ """Bound a real number to be between two values.""" +from math import isfinite +from typing import Union + from gemd.entity.bounds.base_bounds import BaseBounds import gemd.units as units -from typing import Union - class RealBounds(BaseBounds, typ="real_bounds"): """ @@ -21,21 +22,44 @@ class RealBounds(BaseBounds, typ="real_bounds"): """ - def __init__(self, lower_bound=None, upper_bound=None, default_units=None): - self.lower_bound = lower_bound - self.upper_bound = upper_bound - + def __init__(self, lower_bound, upper_bound, default_units): self._default_units = None - self.default_units = default_units + self._lower_bound = None + self._upper_bound = None - if self.lower_bound is None or abs(self.lower_bound) >= float("inf"): - raise ValueError("Lower bound must be given and finite: {}".format(self.lower_bound)) + self.default_units = default_units + self.lower_bound = lower_bound + self.upper_bound = upper_bound - if self.upper_bound is None or abs(self.upper_bound) >= float("inf"): - raise ValueError("Upper bound must be given and finite") + @property + def lower_bound(self) -> float: + """The lower endpoint of the permitted range.""" + return self._lower_bound + + @lower_bound.setter + def lower_bound(self, value: float): + """Set the lower endpoint of the permitted range.""" + if value is None or not isfinite(value): + raise ValueError(f"Lower bound must be given and finite: {value}") + if self.upper_bound is not None and value > self.upper_bound: # Set first + raise ValueError(f"Upper bound ({self.upper_bound}) must be " + f"greater than or equal to lower bound ({value})") + self._lower_bound = float(value) - if self.upper_bound < self.lower_bound: - raise ValueError("Upper bound must be greater than or equal to lower bound") + @property + def upper_bound(self) -> float: + """The upper endpoint of the permitted range.""" + return self._upper_bound + + @upper_bound.setter + def upper_bound(self, value: float): + """Set the upper endpoint of the permitted range.""" + if value is None or not isfinite(value): + raise ValueError(f"Upper bound must be given and finite: {value}") + if value < self.lower_bound: + raise ValueError(f"Upper bound ({value}) must be " + f"greater than or equal to lower bound ({self.lower_bound})") + self._upper_bound = float(value) @property def default_units(self): diff --git a/setup.py b/setup.py index 02e94a51..afbb3375 100644 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ packages.append("") setup(name='gemd', - version='1.17.1', + version='1.18.0', python_requires='>=3.8', url='http://github.com/CitrineInformatics/gemd-python', description="Python binding for Citrine's GEMD data model", diff --git a/tests/entity/bounds/test_integer_bounds.py b/tests/entity/bounds/test_integer_bounds.py index a9b16477..11c50067 100644 --- a/tests/entity/bounds/test_integer_bounds.py +++ b/tests/entity/bounds/test_integer_bounds.py @@ -8,15 +8,22 @@ def test_errors(): """Make sure invalid bounds raise value errors.""" - with pytest.raises(ValueError): + with pytest.raises(TypeError): IntegerBounds() + with pytest.raises(ValueError): + IntegerBounds(float("-inf"), 0) + with pytest.raises(ValueError): IntegerBounds(0, float("inf")) with pytest.raises(ValueError): IntegerBounds(10, 1) + with pytest.raises(ValueError): + bnd = IntegerBounds(0, 1) + bnd.lower_bound = 10 + def test_incompatible_types(): """Make sure that incompatible types aren't contained or validated.""" diff --git a/tests/entity/bounds/test_real_bounds.py b/tests/entity/bounds/test_real_bounds.py index d45bff34..43474a93 100644 --- a/tests/entity/bounds/test_real_bounds.py +++ b/tests/entity/bounds/test_real_bounds.py @@ -57,20 +57,24 @@ def test_contains_incompatible_units(): def test_constructor_error(): """Test that invalid real bounds cannot be constructed.""" - with pytest.raises(ValueError): + with pytest.raises(TypeError): RealBounds() with pytest.raises(ValueError): - RealBounds(0, float("inf"), "meter") + RealBounds(lower_bound=0, upper_bound=float("inf"), default_units="meter") + + with pytest.raises(ValueError): + RealBounds(lower_bound=None, upper_bound=10, default_units='') with pytest.raises(ValueError): - RealBounds(None, 10, '') + RealBounds(lower_bound=0, upper_bound=100, default_units=None) with pytest.raises(ValueError): - RealBounds(0, 100) + RealBounds(lower_bound=100, upper_bound=0, default_units="m") with pytest.raises(ValueError): - RealBounds(100, 0, "m") + bnd = RealBounds(lower_bound=0, upper_bound=10, default_units="m") + bnd.lower_bound = 100 def test_type_mismatch(): From 803d654a6cacf6d80a16ffe67bc087e692676ee5 Mon Sep 17 00:00:00 2001 From: Ken Kroenlein Date: Wed, 10 Jan 2024 15:00:31 -0700 Subject: [PATCH 2/2] PR feedback --- gemd/entity/bounds/integer_bounds.py | 14 ++++----- gemd/entity/bounds/real_bounds.py | 35 +++++++++------------- tests/entity/bounds/test_integer_bounds.py | 20 +++++++++++++ tests/entity/bounds/test_real_bounds.py | 4 +++ 4 files changed, 45 insertions(+), 28 deletions(-) diff --git a/gemd/entity/bounds/integer_bounds.py b/gemd/entity/bounds/integer_bounds.py index 8dbce68d..212fffda 100644 --- a/gemd/entity/bounds/integer_bounds.py +++ b/gemd/entity/bounds/integer_bounds.py @@ -10,7 +10,7 @@ class IntegerBounds(BaseBounds, typ="integer_bounds"): """Bounded subset of the integers, parameterized by a lower and upper bound.""" - def __init__(self, lower_bound, upper_bound): + def __init__(self, lower_bound: int, upper_bound: int): self._lower_bound = None self._upper_bound = None @@ -25,9 +25,9 @@ def lower_bound(self) -> int: @lower_bound.setter def lower_bound(self, value: int): """Set the lower endpoint of the permitted range.""" - if value is None or not isfinite(value): - raise ValueError(f"Lower bound must be given and finite: {value}") - if self.upper_bound is not None and value > self.upper_bound: # Set first + if value is None or not isfinite(value) or int(value) != float(value): + raise ValueError(f"Lower bound must be given, integer and finite: {value}") + if self.upper_bound is not None and value > self.upper_bound: raise ValueError(f"Upper bound ({self.upper_bound}) must be " f"greater than or equal to lower bound ({value})") self._lower_bound = int(value) @@ -40,9 +40,9 @@ def upper_bound(self) -> int: @upper_bound.setter def upper_bound(self, value: int): """Set the upper endpoint of the permitted range.""" - if value is None or not isfinite(value): - raise ValueError(f"Upper bound must be given and finite: {value}") - if value < self.lower_bound: + if value is None or not isfinite(value) or int(value) != float(value): + raise ValueError(f"Upper bound must be given, integer and finite: {value}") + if self.lower_bound is not None and value < self.lower_bound: raise ValueError(f"Upper bound ({value}) must be " f"greater than or equal to lower bound ({self.lower_bound})") self._upper_bound = int(value) diff --git a/gemd/entity/bounds/real_bounds.py b/gemd/entity/bounds/real_bounds.py index cc888975..786888e8 100644 --- a/gemd/entity/bounds/real_bounds.py +++ b/gemd/entity/bounds/real_bounds.py @@ -7,22 +7,9 @@ class RealBounds(BaseBounds, typ="real_bounds"): - """ - Bounded subset of the real numbers, parameterized by a lower and upper bound. - - Parameters - ---------- - lower_bound: float - Lower endpoint. - upper_bound: float - Upper endpoint. - default_units: str - A string describing the units. Units must be present and parseable by Pint. - An empty string can be used for the units of a dimensionless quantity. - - """ + """Bounded subset of the real numbers, parameterized by a lower and upper bound.""" - def __init__(self, lower_bound, upper_bound, default_units): + def __init__(self, lower_bound: float, upper_bound: float, default_units: str): self._default_units = None self._lower_bound = None self._upper_bound = None @@ -41,7 +28,7 @@ def lower_bound(self, value: float): """Set the lower endpoint of the permitted range.""" if value is None or not isfinite(value): raise ValueError(f"Lower bound must be given and finite: {value}") - if self.upper_bound is not None and value > self.upper_bound: # Set first + if self.upper_bound is not None and value > self.upper_bound: raise ValueError(f"Upper bound ({self.upper_bound}) must be " f"greater than or equal to lower bound ({value})") self._lower_bound = float(value) @@ -56,22 +43,28 @@ def upper_bound(self, value: float): """Set the upper endpoint of the permitted range.""" if value is None or not isfinite(value): raise ValueError(f"Upper bound must be given and finite: {value}") - if value < self.lower_bound: + if self.lower_bound is not None and value < self.lower_bound: raise ValueError(f"Upper bound ({value}) must be " f"greater than or equal to lower bound ({self.lower_bound})") self._upper_bound = float(value) @property - def default_units(self): - """Get default units.""" + def default_units(self) -> str: + """ + A string describing the units. + + Units must be present and parseable by Pint. + An empty string can be used for the units of a dimensionless quantity. + """ return self._default_units @default_units.setter - def default_units(self, default_units): + def default_units(self, default_units: str): + """Set the string describing the units.""" if default_units is None: raise ValueError("Real bounds must have units. " "Use an empty string for a dimensionless quantity.") - self._default_units = units.parse_units(default_units) + self._default_units = units.parse_units(default_units, return_unit=False) def contains(self, bounds: Union[BaseBounds, "BaseValue"]) -> bool: # noqa: F821 """ diff --git a/tests/entity/bounds/test_integer_bounds.py b/tests/entity/bounds/test_integer_bounds.py index 11c50067..5f4e6f24 100644 --- a/tests/entity/bounds/test_integer_bounds.py +++ b/tests/entity/bounds/test_integer_bounds.py @@ -11,12 +11,24 @@ def test_errors(): with pytest.raises(TypeError): IntegerBounds() + with pytest.raises(TypeError): + IntegerBounds("0", 10) + with pytest.raises(ValueError): IntegerBounds(float("-inf"), 0) + with pytest.raises(ValueError): + IntegerBounds(0.5, 1) + with pytest.raises(ValueError): IntegerBounds(0, float("inf")) + with pytest.raises(ValueError): + IntegerBounds(0, 0.5) + + with pytest.raises(TypeError): + IntegerBounds(0, "10") + with pytest.raises(ValueError): IntegerBounds(10, 1) @@ -24,6 +36,14 @@ def test_errors(): bnd = IntegerBounds(0, 1) bnd.lower_bound = 10 + with pytest.raises(ValueError): + bnd = IntegerBounds(0, 1) + bnd.upper_bound = -1 + + bnd = IntegerBounds(0, 1) + assert bnd.lower_bound == 0 + assert bnd.upper_bound == 1 + def test_incompatible_types(): """Make sure that incompatible types aren't contained or validated.""" diff --git a/tests/entity/bounds/test_real_bounds.py b/tests/entity/bounds/test_real_bounds.py index 43474a93..cd34ebea 100644 --- a/tests/entity/bounds/test_real_bounds.py +++ b/tests/entity/bounds/test_real_bounds.py @@ -76,6 +76,10 @@ def test_constructor_error(): bnd = RealBounds(lower_bound=0, upper_bound=10, default_units="m") bnd.lower_bound = 100 + bnd = RealBounds(0, 1, "m") + assert bnd.lower_bound == 0.0 + assert bnd.upper_bound == 1.0 + def test_type_mismatch(): """Test that incompatible types cannot be matched against RealBounds."""