Skip to content

Refactors Quantity to have a standard error property #215

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.

Sign up for GitHub

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

Merged
DrPaulSharp merged 1 commit into from
Jun 15, 2026
Merged

Refactors Quantity to have a standard error property #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
60 changes: 28 additions & 32 deletions sasdata/quantities/quantity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1531,9 +1531,9 @@ def apply_operation(
operation(*[history.operation_tree for history in histories], **extra_parameters), references
)

def has_variance(self):
def has_error(self):
for key in self.references:
if self.references[key].has_variance:
if self.references[key].has_error:
return True

return False
Expand Down Expand Up @@ -1570,23 +1570,19 @@ def __init__(
self.hash_value = -1
""" Hash based on value and uncertainty for data, -1 if it is a derived hash value """

self._variance = None
""" Contains the variance if it is data driven """
self._standard_error = standard_error
""" Contains the standard error if it is data driven """

if standard_error is None:
self.hash_value = hash_data_via_numpy(hash_seed, value)
else:
self._variance = standard_error**2
self.hash_value = hash_data_via_numpy(hash_seed, value, standard_error)

self.history = QuantityHistory.variable(self)

self._id_header = id_header
self.name = name

self._id_header = id_header
self.name = name

# TODO: Adding this method as a temporary measure but we need a single
# method that does this.
def with_standard_error(self, standard_error: "Quantity"):
Expand All @@ -1604,17 +1600,21 @@ def with_standard_error(self, standard_error: "Quantity"):
)

@property
def has_variance(self):
return self._variance is not None
def has_error(self):
return self._standard_error is not None

@property
def variance(self) -> "Quantity":
"""Get the variance of this object"""
if self._variance is None:
return Quantity(np.zeros_like(self.value), self.units**2, name=self.name, id_header=self._id_header)
def standard_error(self) -> "Quantity":
"""Get the standard error of this object"""
if self.has_error:
return Quantity(self._standard_error, self.units, name=self.name, id_header=self._id_header)
else:
return Quantity(self._variance, self.units**2)
return Quantity(np.zeros_like(self.value), self.units, name=self.name, id_header=self._id_header)

@property
def variance(self) -> "Quantity":
"""Get the variance of this object"""
return self.standard_error**2
Comment on lines +1614 to +1617

@rprospero rprospero Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's a little odd that variance returns standard_error**2, but the standard_error property returns self._variance_cache**0.5. Wouldn't it be better to cut out the middle man and just return self._variance_cache here?

@DrPaulSharp DrPaulSharp Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can't do this here, because the standard error is defined differently for Quantity compared to DerivedQuantity, as noted above. We could write a variance property for the DerivedQuantity defined as you suggest, but I felt that the code duplication wasn't justified in this case despite the simpler operation. Happy to hear feedback on that point though.


@property
def unique_id(self) -> str:
Expand All @@ -1636,9 +1636,6 @@ def _base62_hash(self) -> str:
current_hash = (current_hash - digit) // 62
return hashed

def standard_deviation(self) -> "Quantity":
return self.variance**0.5

def in_units_of(self, units: Unit) -> QuantityType:
"""Get this quantity in other units"""
if self.units.equivalent(units):
Expand Down Expand Up @@ -1669,16 +1666,15 @@ def in_si(self):
return self.in_units_of(si_units)

def in_units_of_with_standard_error(self, units):
variance = self.variance
units_squared = units**2
standard_error = self.standard_error

if variance.units.equivalent(units_squared):
return self.in_units_of(units), np.sqrt(self.variance.in_units_of(units_squared))
if standard_error.units.equivalent(units):
return self.in_units_of(units), self.standard_error.in_units_of(units)
else:
raise UnitError(f"Target units ({units}) not compatible with existing units ({variance.units}).")
raise UnitError(f"Target units ({units}) not compatible with existing units ({standard_error.units}).")

def in_si_with_standard_error(self):
if self.has_variance:
if self.has_error:
return self.in_units_of_with_standard_error(self.units.si_equivalent())
else:
return self.in_si(), None
Expand Down Expand Up @@ -1837,7 +1833,7 @@ def _array_repr_format(arr: np.ndarray):
def __repr__(self):
if isinstance(self.units, NamedUnit):
value = self.value
error = self.standard_deviation().in_units_of(self.units)
error = self.standard_error.in_units_of(self.units)
unit_string = self.units.symbol

else:
Expand All @@ -1848,12 +1844,12 @@ def __repr__(self):
# Get the array in short form
numeric_string = self._array_repr_format(value)

if self.has_variance:
if self.has_error:
numeric_string += " ± " + self._array_repr_format(error)

else:
numeric_string = f"{value}"
if self.has_variance:
if self.has_error:
numeric_string += f" ± {error}"

return numeric_string + " " + unit_string
Expand Down Expand Up @@ -1912,19 +1908,19 @@ def __init__(self, value: QuantityType, units: Unit, history: QuantityHistory):

self.history = history
self._variance_cache = None
self._has_variance = history.has_variance()
self._has_error = history.has_error()

def to_units_of(self, new_units: Unit) -> "Quantity[QuantityType]":
# TODO: Lots of tests needed for this
return DerivedQuantity(value=self.in_units_of(new_units), units=new_units, history=self.history)

@property
def has_variance(self):
return self._has_variance
def has_error(self):
return self._has_error

@property
def variance(self) -> Quantity:
def standard_error(self) -> Quantity:
if self._variance_cache is None:
self._variance_cache = self.history.variance_propagate(self.units)

return self._variance_cache
return self._variance_cache**0.5

@rprospero rprospero Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This function surprised me slightly and is making me think that I've misunderstood the PR. I'd rather assumed that the standard_error property would just return the value of _standard_error, as defined in line 1573.

@DrPaulSharp DrPaulSharp Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is the case for the standard_error property of the Quantity object, defined on line 1607. Here is the standard_error property of the DerivedQuantity object. Note that the __init__ for this class is:

class DerivedQuantity[QuantityType](Quantity[QuantityType]):
    def __init__(self, value: QuantityType, units: Unit, history: QuantityHistory):
        super().__init__(value, units, standard_error=None)

        self.history = history
        self._variance_cache = None
        self._has_error = history.has_error()

i.e., we call the __init__ for a Quantity with standard_error = None. This is because for a DerivedQuantity we need to propagate errors from the quantity history, rather than read the property.

2 changes: 1 addition & 1 deletion test/sasdataloader/utest_sasdataload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ def test_load_file(test_case: BaseTestCase):
for index, values in expected.items():
for column, expected_value in values.items():
if is_uncertainty(column):
assert loaded._data_contents[column[1::]]._variance[index] == pytest.approx(expected_value**2)
assert loaded._data_contents[column[1::]]._standard_error[index] == pytest.approx(expected_value)
else:
assert loaded._data_contents[column].value[index] == pytest.approx(expected_value)

Expand Down
Loading