diff --git a/documentation/changelog.rst b/documentation/changelog.rst index dd56a80ae..d11d23be6 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -17,6 +17,7 @@ New features Bugfixes ----------- * The CLI command ``flexmeasures show beliefs`` now supports plotting time series data that includes NaN values, and provides better support for plotting multiple sensors that do not share the same unit [see `PR #516 `_] +* Consistent CLI/UI support for asset lat/lng positions up to 7 decimal places (previously the UI rounded to 4 decimal places, whereas the CLI allowed more than 4) [see `PR #522 `_] Infrastructure / Support ---------------------- diff --git a/flexmeasures/api/v2_0/tests/test_api_v2_0_assets.py b/flexmeasures/api/v2_0/tests/test_api_v2_0_assets.py index 7d1a84df6..0a8c445ec 100644 --- a/flexmeasures/api/v2_0/tests/test_api_v2_0_assets.py +++ b/flexmeasures/api/v2_0/tests/test_api_v2_0_assets.py @@ -248,7 +248,7 @@ def test_post_an_asset_with_invalid_data(client, db): in post_asset_response.json["message"]["json"]["capacity_in_mw"][0] ) assert ( - "greater than or equal to -180 and less than or equal to 180" + "Longitude 300.9 exceeds the maximum longitude of 180 degrees." in post_asset_response.json["message"]["json"]["longitude"][0] ) assert "required field" in post_asset_response.json["message"]["json"]["unit"][0] diff --git a/flexmeasures/cli/data_add.py b/flexmeasures/cli/data_add.py index f4e4131f2..6aa0b6e3c 100755 --- a/flexmeasures/cli/data_add.py +++ b/flexmeasures/cli/data_add.py @@ -37,7 +37,13 @@ MissingAttributeException, ) from flexmeasures.data.models.annotations import Annotation, get_or_create_annotation -from flexmeasures.data.schemas import AwareDateTimeField, DurationField, SensorIdField +from flexmeasures.data.schemas import ( + AwareDateTimeField, + DurationField, + LatitudeField, + LongitudeField, + SensorIdField, +) from flexmeasures.data.schemas.sensors import SensorSchema from flexmeasures.data.schemas.units import QuantityField from flexmeasures.data.schemas.generic_assets import ( @@ -241,12 +247,12 @@ def add_asset_type(**args): @click.option("--name", required=True) @click.option( "--latitude", - type=float, + type=LatitudeField(), help="Latitude of the asset's location", ) @click.option( "--longitude", - type=float, + type=LongitudeField(), help="Longitude of the asset's location", ) @click.option("--account-id", type=int, required=True) diff --git a/flexmeasures/data/schemas/__init__.py b/flexmeasures/data/schemas/__init__.py index 0ae18d982..96cd296f4 100644 --- a/flexmeasures/data/schemas/__init__.py +++ b/flexmeasures/data/schemas/__init__.py @@ -1,3 +1,4 @@ +from .assets import LatitudeField, LongitudeField # noqa F401 from .generic_assets import GenericAssetIdField as AssetIdField # noqa F401 from .sensors import SensorIdField # noqa F401 from .times import AwareDateTimeField, DurationField # noqa F401 diff --git a/flexmeasures/data/schemas/assets.py b/flexmeasures/data/schemas/assets.py index db4bd6e31..79c29a2f7 100644 --- a/flexmeasures/data/schemas/assets.py +++ b/flexmeasures/data/schemas/assets.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from marshmallow import validates, ValidationError, validates_schema, fields, validate from flexmeasures.data import ma @@ -5,6 +7,87 @@ from flexmeasures.data.models.time_series import Sensor from flexmeasures.data.models.user import User from flexmeasures.data.schemas.sensors import SensorSchemaMixin +from flexmeasures.data.schemas.utils import FMValidationError, MarshmallowClickMixin + + +class LatitudeLongitudeValidator(validate.Validator): + """Validator which succeeds if the value passed has at most 7 decimal places.""" + + def __init__(self, *, error: str | None = None): + self.error = error + + def __call__(self, value): + if not round(value, 7) == value: + raise FMValidationError( + "Latitudes and longitudes are limited to 7 decimal places." + ) + return value + + +class LatitudeValidator(validate.Validator): + """Validator which succeeds if the value passed is in the range [-90, 90].""" + + def __init__(self, *, error: str | None = None, allow_none: bool = False): + self.error = error + self.allow_none = allow_none + + def __call__(self, value): + if self.allow_none and value is None: + return + if value < -90: + raise FMValidationError( + f"Latitude {value} exceeds the minimum latitude of -90 degrees." + ) + if value > 90: + raise ValidationError( + f"Latitude {value} exceeds the maximum latitude of 90 degrees." + ) + return value + + +class LongitudeValidator(validate.Validator): + """Validator which succeeds if the value passed is in the range [-180, 180].""" + + def __init__(self, *, error: str | None = None, allow_none: bool = False): + self.error = error + self.allow_none = allow_none + + def __call__(self, value): + if self.allow_none and value is None: + return + if value < -180: + raise FMValidationError( + f"Longitude {value} exceeds the minimum longitude of -180 degrees." + ) + if value > 180: + raise ValidationError( + f"Longitude {value} exceeds the maximum longitude of 180 degrees." + ) + return value + + +class LatitudeField(MarshmallowClickMixin, fields.Float): + """Field that deserializes to a latitude float with max 7 decimal places.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # Insert validation into self.validators so that multiple errors can be stored. + self.validators.insert(0, LatitudeLongitudeValidator()) + self.validators.insert( + 0, LatitudeValidator(allow_none=kwargs.get("allow_none", False)) + ) + + +class LongitudeField(MarshmallowClickMixin, fields.Float): + """Field that deserializes to a longitude float with max 7 decimal places.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # Insert validation into self.validators so that multiple errors can be stored. + self.validators.insert(0, LatitudeLongitudeValidator()) + self.validators.insert( + 0, LongitudeValidator(allow_none=kwargs.get("allow_none", False)) + ) class AssetSchema(SensorSchemaMixin, ma.SQLAlchemySchema): @@ -63,8 +146,8 @@ def validate_soc_constraints(self, data, **kwargs): soc_in_mwh = ma.auto_field() soc_datetime = ma.auto_field() soc_udi_event_id = ma.auto_field() - latitude = fields.Float(required=True, validate=validate.Range(min=-90, max=90)) - longitude = fields.Float(required=True, validate=validate.Range(min=-180, max=180)) + latitude = LatitudeField(allow_none=True) + longitude = LongitudeField(allow_none=True) asset_type_name = ma.auto_field(required=True) owner_id = ma.auto_field(required=True) market_id = ma.auto_field(required=True) diff --git a/flexmeasures/data/schemas/generic_assets.py b/flexmeasures/data/schemas/generic_assets.py index 14d7d81cd..b7aa0b86a 100644 --- a/flexmeasures/data/schemas/generic_assets.py +++ b/flexmeasures/data/schemas/generic_assets.py @@ -1,4 +1,3 @@ -from typing import Optional import json from marshmallow import validates, validates_schema, ValidationError, fields @@ -7,6 +6,7 @@ from flexmeasures.data import ma from flexmeasures.data.models.user import Account from flexmeasures.data.models.generic_assets import GenericAsset, GenericAssetType +from flexmeasures.data.schemas import LatitudeField, LongitudeField from flexmeasures.data.schemas.utils import ( FMValidationError, MarshmallowClickMixin, @@ -35,8 +35,8 @@ class GenericAssetSchema(ma.SQLAlchemySchema): id = ma.auto_field(dump_only=True) name = fields.Str(required=True) account_id = ma.auto_field() - latitude = ma.auto_field() - longitude = ma.auto_field() + latitude = LatitudeField(allow_none=True) + longitude = LongitudeField(allow_none=True) generic_asset_type_id = fields.Integer(required=True) attributes = JSON(required=False) @@ -77,34 +77,6 @@ def validate_account(self, account_id: int): "User is not allowed to create assets for this account." ) - @validates("latitude") - def validate_latitude(self, latitude: Optional[float]): - """Validate optional latitude.""" - if latitude is None: - return - if latitude < -90: - raise ValidationError( - f"Latitude {latitude} exceeds the minimum latitude of -90 degrees." - ) - if latitude > 90: - raise ValidationError( - f"Latitude {latitude} exceeds the maximum latitude of 90 degrees." - ) - - @validates("longitude") - def validate_longitude(self, longitude: Optional[float]): - """Validate optional longitude.""" - if longitude is None: - return - if longitude < -180: - raise ValidationError( - f"Longitude {longitude} exceeds the minimum longitude of -180 degrees." - ) - if longitude > 180: - raise ValidationError( - f"Longitude {longitude} exceeds the maximum longitude of 180 degrees." - ) - class GenericAssetTypeSchema(ma.SQLAlchemySchema): """ diff --git a/flexmeasures/data/schemas/tests/test_latitude_longitude.py b/flexmeasures/data/schemas/tests/test_latitude_longitude.py new file mode 100644 index 000000000..dd695a56b --- /dev/null +++ b/flexmeasures/data/schemas/tests/test_latitude_longitude.py @@ -0,0 +1,84 @@ +import pytest + +from flexmeasures.data.schemas.assets import LatitudeField, LongitudeField +from flexmeasures.data.schemas.utils import ValidationError + + +@pytest.mark.parametrize( + ("input", "exp_deserialization"), + [ + (0, 0), + (0.1234567, 0.1234567), + (-90, -90), + (90, 90), + ], +) +def test_latitude(input, exp_deserialization): + """Testing straightforward cases""" + lf = LatitudeField() + deser = lf.deserialize(input, None, None) + assert deser == exp_deserialization + assert lf.serialize("duration", {"duration": deser}) == round(input, 7) + + +@pytest.mark.parametrize( + ("input", "error_messages"), + [ + ("ninety", ["Not a valid number."]), + (90.01, ["Latitude 90.01 exceeds the maximum latitude of 90 degrees."]), + (0.12345678, ["Latitudes and longitudes are limited to 7 decimal places."]), + ( + -90.00000001, + [ + "Latitude -90.00000001 exceeds the minimum latitude of -90 degrees.", + "Latitudes and longitudes are limited to 7 decimal places.", + ], + ), + ], +) +def test_latitude_field_invalid(input, error_messages): + lf = LatitudeField() + with pytest.raises(ValidationError) as ve: + lf.deserialize(input, None, None) + assert error_messages == ve.value.messages + + +@pytest.mark.parametrize( + ("input", "exp_deserialization"), + [ + (0, 0), + (0.1234567, 0.1234567), + (-180, -180), + (180, 180), + ], +) +def test_longitude(input, exp_deserialization): + """Testing straightforward cases""" + lf = LongitudeField() + deser = lf.deserialize(input, None, None) + assert deser == exp_deserialization + assert lf.serialize("duration", {"duration": deser}) == round(input, 7) + + +@pytest.mark.parametrize( + ("input", "error_messages"), + [ + ("one-hundred-and-eighty", ["Not a valid number."]), + ( + -180.01, + ["Longitude -180.01 exceeds the minimum longitude of -180 degrees."], + ), + ( + -180.00000001, + [ + "Longitude -180.00000001 exceeds the minimum longitude of -180 degrees.", + "Latitudes and longitudes are limited to 7 decimal places.", + ], + ), + ], +) +def test_longitude_field_invalid(input, error_messages): + lf = LongitudeField() + with pytest.raises(ValidationError) as ve: + lf.deserialize(input, None, None) + assert error_messages == ve.value.messages diff --git a/flexmeasures/ui/crud/assets.py b/flexmeasures/ui/crud/assets.py index 52072f72a..e9897c9e5 100644 --- a/flexmeasures/ui/crud/assets.py +++ b/flexmeasures/ui/crud/assets.py @@ -39,12 +39,12 @@ class AssetForm(FlaskForm): name = StringField("Name") latitude = DecimalField( "Latitude", - places=4, + places=None, render_kw={"placeholder": "--Click the map or enter a latitude--"}, ) longitude = DecimalField( "Longitude", - places=4, + places=None, render_kw={"placeholder": "--Click the map or enter a longitude--"}, ) attributes = StringField("Other attributes (JSON)", default="{}") @@ -81,7 +81,13 @@ def process_api_validation_errors(self, api_response: dict): continue for field in list(self._fields.keys()): if field in list(api_response[error_header].keys()): - self._fields[field].errors.append(api_response[error_header][field]) + field_errors = api_response[error_header][field] + if isinstance(field_errors, list): + self._fields[field].errors += api_response[error_header][field] + else: + self._fields[field].errors.append( + api_response[error_header][field] + ) class NewAssetForm(AssetForm):