diff --git a/docs/changes/DM-51676.feature.rst b/docs/changes/DM-51676.feature.rst new file mode 100644 index 00000000..9fe505c1 --- /dev/null +++ b/docs/changes/DM-51676.feature.rst @@ -0,0 +1,2 @@ +Added support for validating units using VOUnit format. +This can be enabled by using the ``--check-vounit`` flag from the command line. diff --git a/python/felis/cli.py b/python/felis/cli.py index 001786f9..c7f22304 100644 --- a/python/felis/cli.py +++ b/python/felis/cli.py @@ -349,6 +349,12 @@ def init_tap_schema( help="Check that at least one column per table is flagged as TAP principal", default=False, ) +@click.option( + "--check-vounit", + is_flag=True, + help="Validate unit fields using the 'vounit' format", + default=False, +) @click.argument("files", nargs=-1, type=click.File()) @click.pass_context def validate( @@ -357,6 +363,7 @@ def validate( check_redundant_datatypes: bool, check_tap_table_indexes: bool, check_tap_principal: bool, + check_vounit: bool, files: Iterable[IO[str]], ) -> None: """Validate one or more felis YAML files. @@ -371,6 +378,8 @@ def validate( Check that every table has a unique TAP table index. check_tap_principal Check that at least one column per table is flagged as TAP principal. + check_vounit + Validate unit fields using the 'vounit' format. files The Felis YAML files to validate. @@ -398,6 +407,7 @@ def validate( "check_redundant_datatypes": check_redundant_datatypes, "check_tap_table_indexes": check_tap_table_indexes, "check_tap_principal": check_tap_principal, + "check_vounit": check_vounit, "id_generation": ctx.obj["id_generation"], }, ) diff --git a/python/felis/datamodel.py b/python/felis/datamodel.py index fdd8772a..d3971b8f 100644 --- a/python/felis/datamodel.py +++ b/python/felis/datamodel.py @@ -85,6 +85,33 @@ """Type for a description, which must be three or more characters long.""" +def _context(info: ValidationInfo, key: str, default: bool = False) -> bool: + """Get a value from the validation context. + + Parameters + ---------- + info + Validation context. + key + Key to look up in the context. + default + Default value to return if the key is not found. All context values + currently in use are boolean, so this should be set to `False` or + `True`. + + Notes + ----- + Since all of the current context values are boolean, this function will + return a default of `False` if the key is not found or if there is no + context available. Should the context parameters be changed in the future + to allow for more complex values, this function will need to be updated to + handle those cases. + """ + if info.context: + return info.context.get(key, default) + return False + + class BaseObject(BaseModel): """Base model. @@ -279,10 +306,15 @@ def check_ivoa_ucd(cls, ivoa_ucd: str) -> str: return validate_ivoa_ucd(ivoa_ucd) @model_validator(mode="after") - def check_units(self) -> Column: + def check_units(self, info: ValidationInfo) -> Column: """Check that the ``fits:tunit`` or ``ivoa:unit`` field has valid units according to astropy. Only one may be provided. + Parameters + ---------- + info + Validation context used to determine if the check is enabled. + Returns ------- `Column` @@ -296,6 +328,7 @@ def check_units(self) -> Column: """ fits_unit = self.fits_tunit ivoa_unit = self.ivoa_unit + check_vounit = _context(info, "check_vounit", False) if fits_unit and ivoa_unit: raise ValueError("Column cannot have both FITS and IVOA units") @@ -303,7 +336,13 @@ def check_units(self) -> Column: if unit is not None: try: - units.Unit(unit) + if check_vounit: + # Enable IVOA unit validation + logger.debug(f"Checking 'vounit' format for column '{self.name}' with value '{unit}'") + units.Unit(unit, format="vounit") + else: + # Use astropy's default unit validation + units.Unit(unit) except ValueError as e: raise ValueError(f"Invalid unit: {e}") diff --git a/tests/data/test_vounit.yaml b/tests/data/test_vounit.yaml new file mode 100644 index 00000000..98ea8e4b --- /dev/null +++ b/tests/data/test_vounit.yaml @@ -0,0 +1,8 @@ +name: vounit_test +description: Schema for testing VOUnit checks +tables: + - name: test_table + columns: + - name: test_column + datatype: float + ivoa:unit: deg_C # Bad unit VOUnit format to trigger validation error diff --git a/tests/test_cli.py b/tests/test_cli.py index 9ec93119..3758645a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -221,6 +221,15 @@ def test_dump_with_invalid_file_extension_error(self) -> None: """Test for ``dump`` command with JSON output.""" run_cli(["dump", TEST_YAML, "out.bad"], expect_error=True) + def test_check_vounit(self) -> None: + """Test for the `--check-vounit` option.""" + # The test schema contains a column with an invalid VOUnit, so + # validation should fail. + run_cli( + ["validate", "--check-vounit", os.path.join(TEST_DIR, "data", "test_vounit.yml")], + expect_error=True, + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_datamodel.py b/tests/test_datamodel.py index a309d36a..c6a96e07 100644 --- a/tests/test_datamodel.py +++ b/tests/test_datamodel.py @@ -237,6 +237,31 @@ def test_timestamp(self) -> None: col = Column(name="testColumn", id="#test_col_id", datatype="timestamp") self.assertEqual(col.votable_xtype, "timestamp") + def test_check_vounit(self) -> None: + """Test validation of the 'vounit' format.""" + # Check that a valid VOUnit is accepted when 'check_vounit' context is + # True. + col_data = { + "name": "testColumn", + "@id": "#test_col_id", + "datatype": "string", + "length": 256, + "ivoa:unit": "m", + } + Column.model_validate(col_data, context={"check_vounit": True}) + + # Check that a unit which is acceptable according to the default + # astropy format but not VOUnit is not accepted when the 'check_vounit' + # context is True. + invalid_col_data = col_data.copy() + invalid_col_data["ivoa:unit"] = "deg_C" + with self.assertRaises(ValidationError): + Column.model_validate(invalid_col_data, context={"check_vounit": True}) + + # Check that a unit which is valid according to the default astropy + # format is accepted when the 'check_vounit' context is False. + Column.model_validate(invalid_col_data, context={"check_vounit": False}) + class TableTestCase(unittest.TestCase): """Test Pydantic validation of the ``Table`` class."""