Tests, param validation, renamed PhotometricErrorModel->ErrorModel

* Renamed PhotometricErrorModel->ErrorModel

* Added parameter tests and validation.

README.md

@@ -63,6 +63,7 @@ If instead you want to calculate errors for LSST year 1, you can pass the `nYrOb
 ```python
 errModel = LsstErrorModel(nYrObs=1)
 ```
+
 ### *Changing the band names*
 
 Another parameter you might want to tweak is the name of the bands.
@@ -99,8 +100,8 @@ This is useful if you do not want to worry about non-detections, but it results
 In addition to `LsstErrorModel`, which comes with the LSST defaults, PhotErr includes `EuclidErrorModel` and `RomanErrorModel`, which come with the Euclid and Roman defaults, respectively.
 Each of these models also have corresponding parameter objects: `EuclidErrorParams` and `RomanErrorParams`.
 
-You can also start with the base error model, `PhotometricErrorModel`, which is not defaulted for any specific survey.
-To instantiate `PhotometricErrorModel`, there are several required arguments that you must supply.
+You can also start with the base error model, `ErrorModel`, which is not defaulted for any specific survey.
+To instantiate `ErrorModel`, there are several required arguments that you must supply.
 To see a list and explanation of these arguments, see the docstring for `ErrorParams`.
 
 # Explanation of the error model

photerr/__init__.py

@@ -3,7 +3,7 @@
 
 from .euclid import EuclidErrorModel, EuclidErrorParams
 from .lsst import LsstErrorModel, LsstErrorParams
-from .model import PhotometricErrorModel
+from .model import ErrorModel
 from .params import ErrorParams
 from .roman import RomanErrorModel, RomanErrorParams
 

photerr/euclid.py

@@ -2,7 +2,7 @@
 from dataclasses import dataclass, field
 from typing import Any, Dict
 
-from photerr.model import PhotometricErrorModel
+from photerr.model import ErrorModel
 from photerr.params import ErrorParams, param_docstring
 
 
@@ -41,7 +41,7 @@ class EuclidErrorParams(ErrorParams):
     )
 
 
-class EuclidErrorModel(PhotometricErrorModel):
+class EuclidErrorModel(ErrorModel):
     """Photometric error model for Euclid."""
 
     def __init__(self, **kwargs: Any) -> None:

photerr/lsst.py

@@ -2,7 +2,7 @@
 from dataclasses import dataclass, field
 from typing import Any, Dict
 
-from photerr.model import PhotometricErrorModel
+from photerr.model import ErrorModel
 from photerr.params import ErrorParams, param_docstring
 
 
@@ -81,7 +81,7 @@ class LsstErrorParams(ErrorParams):
     )
 
 
-class LsstErrorModel(PhotometricErrorModel):
+class LsstErrorModel(ErrorModel):
     """Photometric error model for Euclid."""
 
     def __init__(self, **kwargs: Any) -> None:

photerr/model.py

@@ -7,7 +7,7 @@
 from photerr.params import ErrorParams
 
 
-class PhotometricErrorModel:
+class ErrorModel:
     """Base error model from Ivezic 2019.
 
     References
@@ -45,7 +45,7 @@ def __init__(self, *args: ErrorParams, **kwargs: Any) -> None:
         elif len(args) > 0 and len(kwargs) == 0:
             self._params = args[0]
         elif len(args) == 0 and len(kwargs) > 0:
-            self._params = ErrorParams(**kwargs)
+            self._params = ErrorParams(**kwargs)  # pragma: no cover
         else:
             self._params = args[0].copy()
             self._params.update(**kwargs)
@@ -445,5 +445,5 @@ def getLimitingMags(self, nSigma: float = 5, coadded: bool = True) -> dict:
         # return as a dictionary
         return dict(zip(bands, limiting_mags))
 
-    def __repr__(self) -> str:  # noqa: D105
+    def __repr__(self) -> str:  # pragma: no cover
         return "Photometric error model with parameters:\n\n" + str(self.params)

photerr/params.py

@@ -29,7 +29,7 @@
     Cm : dict
         A band dependent parameter defined in Ivezic 2019
     msky : dict
-        Median zenith sky brightness in each band
+        Median zenith sky brightness in each band, in AB mag / arcsec^2.
     theta : dict
         Median zenith seeing FWHM in arcseconds for each band
     km : dict
@@ -97,6 +97,8 @@
         parameters will be renamed. Additionally, if you are overriding any of the
         default dictionary parameters, you can provide those overrides using *either*
         the old or the new naming scheme.
+    validate : bool; True
+        Whether or not to validate all the parameters.
 
     Notes
     -----
@@ -135,6 +137,35 @@
     Kuijken 2019 - https://arxiv.org/abs/1902.11265
 """
 
+# this dictionary defines the allowed types and values for every parameter
+# we will use it for parameter validation during ErrorParams instantiation
+_val_dict = {
+    # param: [ is dict?, [allowed types], [allowed values], negative allowed? ]
+    "nYrObs": [False, [int, float], [], False],
+    "nVisYr": [True, [int, float], [], False],
+    "gamma": [True, [int, float], [], False],
+    "m5": [True, [int, float], [], True],
+    "tvis": [False, [int, float, type(None)], [], False],
+    "airmass": [False, [int, float, type(None)], [], False],
+    "Cm": [True, [int, float], [], False],
+    "msky": [True, [int, float], [], True],
+    "theta": [True, [int, float], [], False],
+    "km": [True, [int, float], [], False],
+    "sigmaSys": [False, [int, float], [], False],
+    "sigLim": [False, [int, float], [], False],
+    "ndMode": [False, [str], ["flag", "sigLim"], None],
+    "ndFlag": [False, [int, float], [], True],
+    "absFlux": [False, [bool], [], None],
+    "extendedType": [False, [str], ["point", "auto", "gaap"], None],
+    "aMin": [False, [int, float], [], False],
+    "aMax": [False, [int, float], [], False],
+    "majorCol": [False, [str], [], None],
+    "minorCol": [False, [str], [], None],
+    "decorrelate": [False, [bool], [], None],
+    "highSNR": [False, [bool], [], None],
+    "errLoc": [False, [str], ["after", "end", "alone"], None],
+}
+
 
 @dataclass
 class ErrorParams:
@@ -174,24 +205,35 @@ class ErrorParams:
 
     renameDict: InitVar[Dict[str, str]] = None
 
-    def __post_init__(self, renameDict: Union[Dict[str, str], None]) -> None:
+    validate: InitVar[bool] = True
+
+    def __post_init__(
+        self, renameDict: Union[Dict[str, str], None], validate: bool
+    ) -> None:
         """Rename bands and remove duplicate parameters."""
         # if renameDict was provided, rename the bands
         if renameDict is not None:
             self.rename_bands(renameDict)
 
+        # validate the parameters
+        if validate:
+            self._validate_params()
+
         # clean up the dictionaries
         self._clean_dictionaries()
 
-        # if using extended error types, make sure theta is provided for every band
-        if (
-            self.extendedType == "auto" or self.extendedType == "gaap"
-        ) and self.theta.keys() != self.nVisYr.keys():
-            raise ValueError(
-                "If using one of the extended error types "
-                "(i.e. extendedType == 'auto' or 'gaap'), "
-                "then theta must contain an entry for every band."
-            )
+        # if using extended error types,
+        if self.extendedType == "auto" or self.extendedType == "gaap":
+            # make sure theta is provided for every band
+            if set(self.theta.keys()) != set(self.nVisYr.keys()):
+                raise ValueError(
+                    "If using one of the extended error types "
+                    "(i.e. extendedType == 'auto' or 'gaap'), "
+                    "then theta must contain an entry for every band."
+                )
+            # make sure that aMin < aMax
+            elif self.aMin > self.aMax:
+                raise ValueError("aMin must be less than aMax.")
 
     def _clean_dictionaries(self) -> None:
         """Remove unnecessary info from all of the dictionaries.
@@ -207,14 +249,15 @@ def _clean_dictionaries(self) -> None:
 
         # remove the m5 bands from all other parameter dictionaries, and remove
         # bands from all_bands for which we don't have m5 data for
-        ignore_dicts = ["m5", "nVisYr", "gamma", "theta"]
+        ignore_dicts = ["m5", "nVisYr", "gamma"]
         for key, param in self.__dict__.items():
             # get the parameters that are dictionaries
             if isinstance(param, dict) and key not in ignore_dicts:
-                # remove bands that we have explicit m5's for
-                self.__dict__[key] = {
-                    band: val for band, val in param.items() if band not in self.m5
-                }
+                if key != "theta":
+                    # remove bands that we have explicit m5's for
+                    self.__dict__[key] = {
+                        band: val for band, val in param.items() if band not in self.m5
+                    }
 
                 # update the running list of bands that we have sufficient m5 data for
                 all_bands = all_bands.intersection(
@@ -280,18 +323,103 @@ def update(self, *args: dict, **kwargs: Any) -> None:
             else:
                 self.update(**args[0], **kwargs)
 
+        # make sure that all of the keywords are in the class
+        for key in kwargs:
+            if key not in self.__dict__:
+                raise ValueError(
+                    f"'{key}' is not a valid parameter name. "
+                    "Please check the docstring."
+                )
+
         # update parameters from keywords
         safe_copy = self.copy()
         try:
+            # get the init variables
+            renameDict = kwargs.pop("renameDict", None)
+            validate = kwargs.pop("validate", True)
+
+            # update all the other parameters
             for key, param in kwargs.items():
                 setattr(self, key, param)
 
             # call post-init
-            self.__post_init__(renameDict=None)
-        except ValueError as error:
+            self.__post_init__(renameDict=renameDict, validate=validate)
+
+        except Exception as error:
             self.__dict__ = safe_copy.__dict__
-            raise Warning("Aborting update!\n\n" + str(error))
+            raise error
 
     def copy(self) -> ErrorParams:
         """Return a full copy of this ErrorParams instance."""
         return deepcopy(self)
+
+    @staticmethod
+    def _check_single_param(
+        key: str,
+        subkey: str,
+        param: Any,
+        allowed_types: list,
+        allowed_values: list,
+        negative_allowed: bool,
+    ) -> None:
+        """Check that this single parameter has the correct type/value."""
+        name = key if subkey is None else f"{key}.{subkey}"
+
+        if type(param) not in allowed_types:
+            raise TypeError(
+                f"{name} is of type {type(param).__name__}, but should be "
+                f"of type {', '.join(t.__name__ for t in allowed_types)}."
+            )
+        if len(allowed_values) > 0 and param not in allowed_values:
+            raise ValueError(
+                f"{name} has value {param}, but must be one of "
+                f"{', '.join(v for v in allowed_values)}."
+            )
+        if (
+            not negative_allowed
+            and negative_allowed is not None
+            and param is not None
+            and param < 0
+        ):
+            raise ValueError(f"{name} has value {param}, but must be positive!")
+
+    def _validate_params(self) -> None:
+        """Validate parameter types and values."""
+        # this dictionary defines the allowed types and values for every parameter
+
+        # loop over parameter and check against the value dictionary
+        for key, (
+            is_dict,
+            allowed_types,
+            allowed_values,
+            negative_allowed,
+        ) in _val_dict.items():
+            # get the parameter saved under the key
+            param = self.__dict__[key]
+
+            # do we have a dictionary on our hands?
+            if isinstance(param, dict) and not is_dict:
+                raise TypeError(f"{key} should not be a dictionary.")
+            elif not isinstance(param, dict) and is_dict:
+                raise TypeError(f"{key} should be a dictionary.")
+            if is_dict:
+                # loop over contents and check types and values
+                for subkey, subparam in param.items():
+                    self._check_single_param(
+                        key,
+                        subkey,
+                        subparam,
+                        allowed_types,  # type: ignore
+                        allowed_values,  # type: ignore
+                        negative_allowed,  # type: ignore
+                    )
+            else:
+                # check this single value
+                self._check_single_param(
+                    key,
+                    None,  # type: ignore
+                    param,
+                    allowed_types,  # type: ignore
+                    allowed_values,  # type: ignore
+                    negative_allowed,  # type: ignore
+                )

photerr/roman.py

@@ -2,7 +2,7 @@
 from dataclasses import dataclass, field
 from typing import Any, Dict
 
-from photerr.model import PhotometricErrorModel
+from photerr.model import ErrorModel
 from photerr.params import ErrorParams, param_docstring
 
 
@@ -44,7 +44,7 @@ class RomanErrorParams(ErrorParams):
     )
 
 
-class RomanErrorModel(PhotometricErrorModel):
+class RomanErrorModel(ErrorModel):
     """Photometric error model for Roman."""
 
     def __init__(self, **kwargs: Any) -> None:

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "photerr"
-version = "0.2.1"
+version = "1.0.0"
 description = "Photometric error model for astronomical imaging surveys"
 authors = ["John Franklin Crenshaw <[email protected]>"]
 readme = "README.md"