From 57f98afef9b540f9a74da51d7869e2908473bf7d Mon Sep 17 00:00:00 2001 From: 0x1r1s2 Date: Tue, 25 Aug 2026 19:23:31 -0400 Subject: [PATCH 1/2] Fix DunderMixin hash equality contract for dictionary merging purposes --- steer_core/Mixins/Dunder.py | 17 +++++++++-------- test/test_dunder_mixin.py | 15 ++++++++++----- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/steer_core/Mixins/Dunder.py b/steer_core/Mixins/Dunder.py index b6fcd9f..6a2d4b3 100644 --- a/steer_core/Mixins/Dunder.py +++ b/steer_core/Mixins/Dunder.py @@ -252,15 +252,17 @@ def __eq__(self, other): return True def __hash__(self): - """Hash based on object identity. + """Return a stable hash compatible with value-based equality. - Because the objects using this mixin are mutable, a content-based - hash would be unstable. Using ``id()`` means that two distinct - objects that compare equal via ``__eq__`` will have different - hashes -- avoid using these objects as ``set`` members or ``dict`` - keys when value-based identity matters. + Instances using this mixin are mutable, so hashing their property + values would make their hash unstable. A constant hash preserves + the requirement that objects which compare equal have equal hashes + while allowing ``dict`` and ``set`` lookups to use ``__eq__``. + + The tradeoff is that hash-based lookups among these objects are + linear rather than constant-time. """ - return hash(id(self)) + return 0 def __str__(self): """ @@ -276,4 +278,3 @@ def __repr__(self): Official string representation of the instance. """ return self.__str__() - diff --git a/test/test_dunder_mixin.py b/test/test_dunder_mixin.py index f7ac1e4..9f9da19 100644 --- a/test/test_dunder_mixin.py +++ b/test/test_dunder_mixin.py @@ -33,12 +33,17 @@ def test_different_name(self, sample_obj): class TestDunderHash: - def test_hash_is_identity_based(self, sample_obj): - assert hash(sample_obj) == hash(id(sample_obj)) + def test_equal_objects_have_equal_hashes(self, sample_obj): + equivalent = SampleObject(name="test", value=1.0) - def test_different_objects_different_hash(self, sample_obj): - other = SampleObject() - assert hash(sample_obj) != hash(other) + assert sample_obj == equivalent + assert hash(sample_obj) == hash(equivalent) + + def test_equivalent_object_resolves_dict_key(self, sample_obj): + equivalent = SampleObject(name="test", value=1.0) + values = {sample_obj: 7} + + assert values[equivalent] == 7 class TestDunderStr: From 8dc9703eb0d6d4b419a82af9fb6b6d387d9bf721 Mon Sep 17 00:00:00 2001 From: Nicholas Siemons Date: Mon, 14 Sep 2026 14:54:39 -0700 Subject: [PATCH 2/2] added_strictly_to_validate_pos_float --- CHANGELOG.md | 20 ++++++++++++++++++++ steer_core/Mixins/TypeChecker.py | 29 +++++++++++++++++++++++++---- steer_core/__init__.py | 2 +- test/test_typechecker_mixin.py | 23 +++++++++++++++++++++++ 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d420199..d14975b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.2.23] - 2026-09-14 + +### Fixed +- **`ValidationMixin.validate_positive_float` accepted `NaN` and `inf`.** The + check was `if value < 0`, which both slip past (`nan < 0` is False), so a + non-finite value passed validation and was committed to whatever property was + being set — poisoning every quantity derived from it with no exception + anywhere. Non-finite values now raise `ValueError`. Downstream: any setter + that reads a cached float and scales it (e.g. + `_Electrode.reversible_areal_capacity` in `steer-opencell-design`) could + silently store `nan`. + +### Added +- `validate_positive_float(value, name, strictly=False)` takes a `strictly` + flag, mirroring `validate_positive_int`. With `strictly=True` a value of 0.0 + is rejected, which callers need when zero would brick the property being set + (a zeroed denominator that can never be scaled back up). The default stays + non-strict (`>= 0`) because many callers legitimately pass 0.0 — zero + insulation width, zero gap, zero electrolyte overfill. + ## [0.2.22] - 2026-08-18 ### Fixed diff --git a/steer_core/Mixins/TypeChecker.py b/steer_core/Mixins/TypeChecker.py index b5ab0a3..a72e70c 100644 --- a/steer_core/Mixins/TypeChecker.py +++ b/steer_core/Mixins/TypeChecker.py @@ -135,20 +135,41 @@ def validate_datum(datum: np.ndarray) -> None: raise TypeError("All coordinates in datum must be numbers.") @staticmethod - def validate_positive_float(value: float, name: str) -> None: + def validate_positive_float(value: float, name: str, strictly: bool = False) -> None: """Validate that a value is a positive float. Args: value: The value to validate. name: The name of the parameter for error messages. + strictly: If True, value must be strictly positive (> 0). Defaults to + False (>= 0), because many callers legitimately pass 0.0 (zero + insulation width, zero gap, zero overfill). Mirrors + :meth:`validate_positive_int`, which defaults to strict because + its callers are counts. Raises: - ValueError: If the value is not a positive float. + TypeError: If the value is not a number. + ValueError: If the value is NaN or infinite, or does not meet the + positivity requirement. + + Examples: + >>> ValidationMixin.validate_positive_float(5.0, 'width') # OK + >>> ValidationMixin.validate_positive_float(0.0, 'width') # OK (non-strict default) + >>> ValidationMixin.validate_positive_float(0.0, 'width', strictly=True) # ValueError + >>> ValidationMixin.validate_positive_float(-1.0, 'width') # ValueError + >>> ValidationMixin.validate_positive_float(float('nan'), 'width') # ValueError + >>> ValidationMixin.validate_positive_float(float('inf'), 'width') # ValueError """ if not isinstance(value, (int, float, np.int64, np.float64)): raise TypeError(f"{name} must be a number. Provided: {type(value).__name__}.") - if value < 0: - raise ValueError(f"{name} must be a positive float. Provided: {value}.") + if not np.isfinite(value): + raise ValueError(f"{name} must be a finite number. Provided: {value}.") + if strictly: + if value <= 0: + raise ValueError(f"{name} must be a strictly positive float (> 0). Provided: {value}.") + else: + if value < 0: + raise ValueError(f"{name} must be a positive float. Provided: {value}.") @staticmethod def validate_positive_int(value: int, name: str, strictly: bool = True) -> None: diff --git a/steer_core/__init__.py b/steer_core/__init__.py index c36b195..ff415e9 100644 --- a/steer_core/__init__.py +++ b/steer_core/__init__.py @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: 2024-2026 Stanford University # SPDX-License-Identifier: AGPL-3.0-or-later -__version__ = "0.2.22" +__version__ = "0.2.23" from .Mixins.Colors import ColorMixin from .Mixins.Coordinates import CoordinateMixin diff --git a/test/test_typechecker_mixin.py b/test/test_typechecker_mixin.py index 14fe97e..8ab9092 100644 --- a/test/test_typechecker_mixin.py +++ b/test/test_typechecker_mixin.py @@ -124,6 +124,29 @@ def test_non_numeric_raises(self): def test_numpy_int64(self): ValidationMixin.validate_positive_float(np.int64(5), "val") + def test_nan_raises(self): + with pytest.raises(ValueError): + ValidationMixin.validate_positive_float(float("nan"), "val") + + def test_infinity_raises(self): + with pytest.raises(ValueError): + ValidationMixin.validate_positive_float(float("inf"), "val") + + def test_negative_infinity_raises(self): + with pytest.raises(ValueError): + ValidationMixin.validate_positive_float(float("-inf"), "val") + + def test_numpy_nan_raises(self): + with pytest.raises(ValueError): + ValidationMixin.validate_positive_float(np.float64("nan"), "val") + + def test_strictly_rejects_zero(self): + with pytest.raises(ValueError): + ValidationMixin.validate_positive_float(0.0, "val", strictly=True) + + def test_strictly_accepts_positive(self): + ValidationMixin.validate_positive_float(5.0, "val", strictly=True) + class TestValidatePositiveInt: