Issue 504 prevent silently rounding off latitude and longitude values…

… on asset page (#522)

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).


* Do not round latitude/longitude values in AssetForm

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Implement validated longitude and longitude fields that round to 7 decimal places

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Use latitude and longitude fields in CLI

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Raise rather than round

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Use latitude and longitude fields in API

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Extend rather than append list of error messages returns from internal API

Signed-off-by: F.N. Claessen <felix@seita.nl>

* black

Signed-off-by: F.N. Claessen <felix@seita.nl>

* flake8

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Fix tests

Signed-off-by: F.N. Claessen <felix@seita.nl>

* changelog entry

Signed-off-by: F.N. Claessen <felix@seita.nl>

Signed-off-by: F.N. Claessen <felix@seita.nl>

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 <http://www.github.com/FlexMeasures/flexmeasures/pull/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 <http://www.github.com/FlexMeasures/flexmeasures/pull/522>`_]
 
 Infrastructure / Support
 ----------------------

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]

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)

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

flexmeasures/data/schemas/assets.py

@@ -1,10 +1,93 @@
+from __future__ import annotations
+
 from marshmallow import validates, ValidationError, validates_schema, fields, validate
 
 from flexmeasures.data import ma
 from flexmeasures.data.models.assets import Asset, AssetType
 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)

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):
     """

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

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):