processed review

Co-authored-by: Jochem-L <[email protected]>

README.md

@@ -1,7 +1,7 @@
 # Coordinate Transformation API
 
-RESTful Coordinate Transformation API offering NSGI approved transformations for
-the Netherlands. Build on top of pyproj and FastAPI.
+RESTful Coordinate Transformation API offering NSGI defined and NSGI recommended
+transformations for the Netherlands. Build on top of pyproj and FastAPI.
 
 ## Assumptions
 
@@ -20,23 +20,19 @@ Pyproj with default configuration is not capable in performing the right
 transformations, because our primary transformations layer on the following
 transformation grids:
 
-Variant 1:
+Recommended RDNAPTRANS(TM)2018 variant:
 
-1. <https://cdn.proj.org/nl_nsgi_nlgeo2018.tif>
-1. <https://cdn.proj.org/nl_nsgi_rdcorr2018.tif>
+- <https://cdn.proj.org/nl_nsgi_nlgeo2018.tif>
+- <https://cdn.proj.org/nl_nsgi_rdcorr2018.tif>
 
-The recommended variant.
-
-Variant 2:
-
-1. <https://cdn.proj.org/nl_nsgi_rdtrans2018.tif>
+And the geoid for BESTRANS2020
 
 These transformation grids need to be downloaded from the
 [PROJ.org Datumgrid CDN](https://cdn.proj.org/) and put in the correct
 directory. This can be done in a couple of ways.
 
 1. Enable PROJ_NETWORK environment variable
-1. Edit proj.ini file by setting `network = on`
+2. Edit proj.ini file by setting `network = on`
 
 These will download the necessary files to a cache so they can be use for the
 correct transformation. But this requires a network connection, preferable we

src/coordinate_transformation_api/crs_transform.py

@@ -85,7 +85,7 @@ def my_fun(
     return my_fun
 
 
-# Strip height/elevation from coordinate
+# Strip height from coordinate
 # [1,2,3] -> [1,2]
 def get_remove_json_height_fun() -> Callable[[CoordinatesType], tuple[float, ...]]:
     def remove_json_height_fun(
@@ -229,7 +229,7 @@ def get_bbox_from_coordinates(coordinates: Any) -> BBox:  # noqa: ANN401
         return min(x), min(y), min(z), max(x), max(y), max(z)
     else:
         raise ValueError(
-            f"expected length of coordinate tuple is either 2 or 3, got {len(coordinate_tuples)}"
+            f"expected dimension of coordinates is either 2 or 3, got {len(coordinate_tuples)}"
         )
 
 
@@ -242,7 +242,7 @@ def exclude_transformation(source_crs_str: str, target_crs_str: str) -> bool:
 
 
 def needs_epoch(tf: Transformer) -> bool:
-    # Currently the time dependent & specific operation method code are hardcoded
+    # Currently the time-dependent & specific operation method code are hardcoded
     # These are extracted from the 'coordinate_operation_method' table in the proj.db
     static_coordinate_operation_methode_time_dependent = [
         "1053",
@@ -304,18 +304,18 @@ def get_transformer(
             target_crs=str(t_crs),
         )
 
-    # When no input epoch is given we need to check that we don't perform an time dependent transformation. If we do
-    # the transformation will be done with a default epoch value, which isn't correct. So we need to search for the "best"
-    # transformation that doesn't include a time dependent operation methode.
+    # When no input epoch is given we need to check that we don't perform an time-dependent transformation. Otherwise
+    # the transformation would be done with a default epoch value, which isn't correct. So we need to search for the "best"
+    # transformation that doesn't include a time-dependent operation methode.
     if epoch is None:
         for tf in tfg.transformers:
             if needs_epoch(tf) is not True:
                 return tf
 
-    # When reaching this point and the 'only' transformation available is an time dependent transformation, but no epoch is provided
-    # we don't want to use the 'default' epoch associated with the transformation but the won't execute the transformation. Because
-    # when the transformation is done with the default epoch (e.i 2010) but the coords are from 2023 the deviation will be too large.
-    # Resulting in wrong result, there for we prefer giving an exception, rather then a wrong result.
+    # When reaching this point and the 'only' transformation available is an time-dependent transformation, but no epoch is provided,
+    # we don't want to use the 'default' epoch associated with the transformation. Instead, we won't execute the transformation. Because
+    # when the transformation is done with the default epoch (e.g. 2010), but the coords are from 2023 this
+    # results in wrong results. We prefer giving an exception, rather than a wrong result.
     if needs_epoch(tfg.transformers[0]) is True and epoch is None:
         raise TransformationNotPossibleError(
             src_crs=str(s_crs),
@@ -355,11 +355,11 @@ def my_round(val: float, precision: int | None) -> float | int:
 
     transformer = get_transformer(source_crs, target_crs, epoch)
 
-    # We need to do something special for transformation targetting a Compound CRS, like NAP or a LAT-NL height
-    # - RDNAP (EPSG:7415)
+    # We need to do something special for transformation targetting a Compound CRS of 2D coordinates with another height system, like NAP or a LAT height
+    # - RD + NAP (EPSG:7415)
     # - ETRS89 + NAP (EPSG:9286)
     # - ETRS89 + LAT-NL (EPSG:9289)
-    # These transformations need to be splitted between a horizontal and vertical transformation.
+    # These transformations need to be splitted in a horizontal and vertical transformation.
     if (
         transformer.target_crs is not None
         and transformer.target_crs.type_name == "Compound CRS"
@@ -402,9 +402,7 @@ def transform_crs(val: CoordinatesType) -> tuple[float, ...]:
                 and TWO_DIMENSIONAL > dim > THREE_DIMENSIONAL
             ):
                 # check so we can safely cast to tuple[float, float], tuple[float, float, float]
-                raise ValueError(
-                    f"number of dimensions of target-crs should be 2 or 3, is {dim}"
-                )
+                raise ValueError(f"dimension of target-crs should be 2 or 3, is {dim}")
             val = cast(tuple[float, float] | tuple[float, float, float], val[0:dim])
 
             # TODO: fix epoch handling, should only be added in certain cases
@@ -423,7 +421,7 @@ def transform_crs(val: CoordinatesType) -> tuple[float, ...]:
             # GeoJSON and CityJSON by definition has coordinates always in lon-lat-height (or x-y-z) order. Transformer has been created with `always_xy=True`,
             # to ensure input and output coordinates are in in lon-lat-height (or x-y-z) order.
             # Regarding the epoch: this is stripped from the result of the transformer. It's used as a input parameter for the transformation but is not
-            # 'needed' in the result, because there is no conversion of time, e.i. a epoch value of 2010.0 will stay 2010.0 in the result. Therefor the result
+            # 'needed' in the result, because there is no conversion of time, e.i. an epoch value of 2010.0 will stay 2010.0 in the result. Therefor the result
             # of the transformer is 'stripped' with [0:dim]
             output = tuple(
                 [

src/coordinate_transformation_api/main.py

@@ -258,7 +258,7 @@ async def crs(crs_id: str) -> Crs | Response:
 async def conformance() -> Conformance:
     return Conformance(
         conformsTo=[
-            # does not conform to fully to the following standards, but effort has been made to conform as much as possible
+            # does not conform fully to the following standards, but effort has been made to conform as much as possible
             # "https://docs.ogc.org/is/19-072/19-072.html",
             # "https://gitdocumentatie.logius.nl/publicatie/api/adr/",
         ]

src/coordinate_transformation_api/models.py

@@ -73,7 +73,7 @@ def __init__(
 
 class DeviationOutOfBboxError(DataValidationError):
     type_str = "nsgi.nl/deviation-data-outside-bbox"
-    title = "Data Outside Bounding Box when Using Deviation"
+    title = "Data Outside Bounding Box when Using a max. Deviation"
     pass
 
 
@@ -204,6 +204,6 @@ def get_x_unit_crs(self: "Crs") -> str:
         unit_name = axe.unit_name
         if unit_name not in ["degree", "metre"]:
             raise ValueError(
-                f"Unexpected unit in x axis (x, e, lon) CRS {self.crs_auth_identifier} - expected values: degree, meter, actual value: {unit_name}"
+                f"Unexpected unit of first axis (x, E, lon) of CRS {self.crs_auth_identifier} - expected values: degree, meter, actual value: {unit_name}"
             )
         return unit_name

src/coordinate_transformation_api/settings.py

@@ -71,7 +71,7 @@ class AppSettings(BaseSettings):
     precision: int = Field(
         alias="PRECISION",
         default=4,
-        description="precision for output coordinates in GeoJSON format for CRS in meters, precision for degrees based CRS is PRECISION+5",
+        description="number of decimals for output coordinates in GeoJSON format for CRS in meters, number of decimals for degrees-based CRS is PRECISION+5",
     )
     base_url: str = Field(
         alias="BASE_URL",
@@ -82,7 +82,7 @@ class AppSettings(BaseSettings):
     cors_allow_origins: Union[list[AnyHttpUrl], CorsAllOrNone] = Field(
         alias="CORS_ALLOW_ORIGINS",
         default=None,
-        description="CORS origins, either a comma separated list of HTTPS urls of the value `*` to allow CORS on all origins",
+        description="Cross-Origin Resource Sharing (CORS), either a comma separated list of HTTPS urls of the value `*` to allow CORS on all origins",
     )
     access_log: bool = Field(
         alias="ACCESS_LOG",

src/coordinate_transformation_api/util.py

@@ -164,7 +164,7 @@ def density_check_request_body(
     max_segment_deviation: float | None,
     max_segment_length: float | None,
 ) -> CrsFeatureCollection:
-    """Run density check with langelijnenadvies implementatie, by running density check in DENSIFY_CRS."""
+    """Run density check with geodense implementation, by running density check in DENSIFY_CRS."""
     _geom_type_check(body)
     if max_segment_deviation is not None:
         bbox_check_deviation_set(body, source_crs, max_segment_deviation)
@@ -181,7 +181,7 @@ def density_check_request_body(
     if transform:
         crs_transform(
             body, source_crs, transform_crs
-        )  # !NOTE: crs_transform is required for langelijnen advies implementatie
+        )  # !NOTE: crs_transform is required for density_check and densify
     c = DenseConfig(CRS.from_authority(*DENSIFY_CRS_2D.split(":")), max_segment_length)
     failed_line_segments = density_check_geojson_object(body, c)
 
@@ -207,7 +207,7 @@ def densify_request_body(
     max_segment_deviation: float | None,
     max_segment_length: float | None,
 ) -> None:
-    """densify request body according to langelijnenadvies by densifying in DENSIFY_CRS
+    """densify request body according to geodense by densifying in DENSIFY_CRS
 
     Args:
         body (Feature | FeatureCollection | _GeometryBase | GeometryCollection): request body to transform, will be transformed in place
@@ -270,7 +270,7 @@ def init_oas(crs_config) -> tuple[dict, str, str]:
             }
             security: dict = {"security": [{"APIKeyHeader": []}]}
             if app_settings.example_api_key is not None:
-                api_key_description = f"\n\nThe Demo API key is `{app_settings.example_api_key}` and is intended for exploratory use of the API only. "
+                api_key_description = f"\n\nThe Demo API key is `{app_settings.example_api_key}` and is intended for exploratory use of the API only. This key may stop working without warning."
                 oas["info"]["description"] = (
                     oas["info"]["description"] + api_key_description
                 )
@@ -384,7 +384,7 @@ def validate_crs_transformed_geojson(body: GeojsonObject) -> None:
 
 
 def remove_height_when_inf_geojson(body: GeojsonObject) -> GeojsonObject:
-    # Seperated check on inf height/elevation
+    # Seperated check on inf height
     validate_json_height_fun = get_json_height_contains_inf_fun()
     contains_inf_height: Nested[bool] = apply_function_on_geojson_geometries(
         body, validate_json_height_fun