Add parameter information to /collections and /locations endpoint #35

lukas-phaf · 2024-02-13T16:14:10Z

Also improved the data endpoints to return the same parameter info as the /collections and /locations endpoints.

TODO:

Fix integration tests
The code is currently hardcoded to get a specific range of data, which corresponds to 1 row in the KNMI dataset. This should be removed and replaced by the "get last value" functionality that Jo is working on (Issue 40 getobservations latest #41)

github-actions · 2024-02-13T16:14:42Z

API Unit Test Coverage Report

File	Stmts	Miss	Cover	Missing
\_\_init\_\_.py	0	0	100%
datastore_pb2.py	58	46	21%	24–69
datastore_pb2_grpc.py	43	23	47%	37–52, 85–87, 92–94, 99–101, 106–108, 112–136, 174, 191, 208, 225
dependencies.py	48	18	62%	16, 26–27, 34, 41, 52, 62–69, 77–84
grpc_getter.py	16	8	50%	15–16, 20–23, 27–29
locustfile.py	15	15	0%	1–31
main.py	22	3	86%	27, 37, 47
metadata_endpoints.py	34	12	65%	24, 42–103, 107
custom_geo_json
edr_feature_collection.py	6	0	100%
formatters
\_\_init\_\_.py	7	0	100%
covjson.py	50	4	92%	71, 86–89
routers
\_\_init\_\_.py	0	0	100%
edr.py	88	37	58%	45–104, 132–144, 173, 184–190, 229–246
records.py	0	0	100%
TOTAL	387	166	57%

API Unit Test Coverage Summary

Tests	Skipped	Failures	Errors	Time
16	0 💤	9 ❌	0 🔥	2.278s ⏱️

api/routers/edr.py

api/metadata_endpoints.py

datastore/data-loader/client_knmi_station.py

api/metadata_endpoints.py

…s endpoint in python.

…ontainer, not in the volume.

Used nametuple for clarity.

…the loader to have a unique parameter ID and make the parameters distinguishable.

…l of the parameters for the locations.

…t as step to the ci test-datastore action.

Teddy-1000

Left a comment regarding the EDRFeatureCollection, otherwise the PR looks good.

api/routers/edr.py

…roved them to make them unique and more accurate.

lukas-phaf · 2024-03-04T16:06:15Z

...e/integration-test/response/collection/area/200/data_within_an_area_with_two_parameters.json

@@ -52,35 +52,43 @@
                ]
            },
            "parameters": {
-                "relative_humidity_2.0_mean_PT1M": {
+                "wind_speed_10_mean_PT10M": {


What is the order that is used? As this is not alphabetical. Is it fixed? Collection endpoints does seem to be ordered? Should really be the same.

lukas-phaf · 2024-03-04T16:06:42Z

api/formatters/covjson.py

+    # param_id = ts_mdata.parameter_name
+    # standard_name = ts_mdata.standard_name
+    # title = ts_mdata.title
+    # unit = ts_mdata.unit


These lines can go, right?

lukas-phaf · 2024-03-06T08:53:39Z

.github/workflows/ci.yml

+      - name: Test client runs without errors
+        run: DYNAMICTIME=false LOTIME=1000-01-01T00:00:00Z HITIME=9999-12-31T23:59:59Z docker compose run --rm client
+


This is now run twice (see a couple of lines down). Any idea why?

As the client is part of the profile test in the docker compose. I ran into the problem that the integration tests failed because locally they had the client, but in the pipeline not.
I wanted to move it upwards in the pipeline. But instead of moving, I copied it.

Originally client did not leave any data... but now it does, and the integration test depend on this data? Do we want that?

I had my doubts about that too, the reason that I added it is that it shows current issues such as the empty string as default for gRPC instead of NULL. But I can change it as well to not include it.

Teddy-1000 · 2024-03-06T09:46:52Z

api/metadata_endpoints.py

+
+    for obs in ts_response.observations:
+        parameter = Parameter(
+            description=obs.ts_mdata.title,


Sorry I did not catch this earlies. But I do not think ts_mdata.title can be used here, as it will most likely be different for every time series. What we really need is the standard_name description from nerc vocab.

We also run in to the same issue in /locations

So you are hitting the "mutiple definitions" 500 error?

Shall we just use standard_name for description for now?
I am guessing that obs.ts_mdata.instrument for label (3 lines down) is also not correct.

To get back to this, I don't think the description from the nerc vocab would do, as that is a description of the standard_name, not of this specific parameter.

All the different air_temperatures now have the same description, which doesn't make to much sense. Some measure it at grass height, some at 2.0m. And there can have different average times. The would all be described as "Air temperature is the bulk temperature of the air, not the surface (skin) temperature.".

What we need in the metadata is a field that describes the parameter as measured, independent of the station.

BTW, why are all the ts_mdata.title different in your case. Do you include the station name in them?

jo-asplin-met-no reviewed Feb 14, 2024

View reviewed changes

api/routers/edr.py Show resolved Hide resolved

jo-asplin-met-no reviewed Feb 14, 2024

View reviewed changes

api/routers/edr.py Outdated Show resolved Hide resolved

jo-asplin-met-no reviewed Feb 14, 2024

View reviewed changes

api/routers/edr.py Outdated Show resolved Hide resolved

lukas-phaf changed the title ~~Add parameter information to /locations endpoint~~ Add parameter information to /collections and /locations endpoint Feb 15, 2024

lukas-phaf commented Feb 16, 2024

View reviewed changes

api/metadata_endpoints.py Outdated Show resolved Hide resolved

Jeffrey-Vervoort-KNMI force-pushed the issue_20-locations_parameters-python branch 3 times, most recently from 573778f to fc4f794 Compare February 23, 2024 12:12

Jeffrey-Vervoort-KNMI reviewed Feb 23, 2024

View reviewed changes

datastore/data-loader/client_knmi_station.py Outdated Show resolved Hide resolved

Jeffrey-Vervoort-KNMI force-pushed the issue_20-locations_parameters-python branch 4 times, most recently from 4822fe6 to 79c4eba Compare February 28, 2024 16:10

Jeffrey-Vervoort-KNMI reviewed Feb 28, 2024

View reviewed changes

api/metadata_endpoints.py Show resolved Hide resolved

Jeffrey-Vervoort-KNMI marked this pull request as ready for review February 28, 2024 16:17

Jeffrey-Vervoort-KNMI and others added 14 commits February 29, 2024 08:16

Working on an implementation for all parameter names for the location…

d880c91

…s endpoint in python.

Check if GetTSAGRequest provides enough information.

4936f09

cleanup

acecfe2

Cleanup remove code for local testing via the IDE

2b87771

Basic functionality working.

1c74979

Added some error checking.

32d5d5c

Add parameters to /collections.

0833bfa

Fix database volume location, as the data was stored in the running c…

e7283db

…ontainer, not in the volume.

Align parameter output with /collections and /locations endpoints.

fa1de06

Used nametuple for clarity.

Fix "rel" self versus data in collections metadata.

80e1338

Simplify code.

a515bd1

Remove duplication.

273a010

Change instrument to the new parameter name.

65c349e

Update integration tests, apply new parameter name and add a hack to …

bd758c2

…the loader to have a unique parameter ID and make the parameters distinguishable.

Jeffrey-Vervoort-KNMI added 3 commits February 29, 2024 08:16

Replace the workaround with the temporal mode latest for the retrieva…

0a19aea

…l of the parameters for the locations.

Fix the other endpoints and their integration tests.

af0c9af

Apply pre-commit hook.

0fa9854

Jeffrey-Vervoort-KNMI force-pushed the issue_20-locations_parameters-python branch from 79c4eba to 0fa9854 Compare February 29, 2024 07:33

For consistency with the profile test in the docker compose add clien…

013bd63

…t as step to the ci test-datastore action.

Jeffrey-Vervoort-KNMI force-pushed the issue_20-locations_parameters-python branch from e6eed8e to 013bd63 Compare February 29, 2024 08:25

Teddy-1000 approved these changes Feb 29, 2024

View reviewed changes

api/routers/edr.py Show resolved Hide resolved

This was linked to issues Feb 29, 2024

Implement /locations endpoint #20

Closed

[API] Reuse gRPC channels and stubs where possible #38

Closed

[API] Add parameter info to /collections metadata endpoint. #39

Closed

Jeffrey-Vervoort-KNMI force-pushed the issue_20-locations_parameters-python branch from cff9b4a to 039c706 Compare March 4, 2024 11:24

Looked up values for the KNMI parameters that were not unique and imp…

9e766ea

…roved them to make them unique and more accurate.

Jeffrey-Vervoort-KNMI force-pushed the issue_20-locations_parameters-python branch from 039c706 to 9e766ea Compare March 4, 2024 12:24

Add ToDo EDR Feature Collection.

4056f7d

Jeffrey-Vervoort-KNMI merged commit 097aa15 into main Mar 4, 2024
6 checks passed

lukas-phaf commented Mar 4, 2024

View reviewed changes

lukas-phaf commented Mar 6, 2024

View reviewed changes

Teddy-1000 reviewed Mar 6, 2024

View reviewed changes

Jeffrey-Vervoort-KNMI deleted the issue_20-locations_parameters-python branch March 14, 2024 13:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add parameter information to /collections and /locations endpoint #35

Add parameter information to /collections and /locations endpoint #35

lukas-phaf commented Feb 13, 2024 •

edited by Jeffrey-Vervoort-KNMI

Loading

github-actions bot commented Feb 13, 2024 •

edited

Loading

Teddy-1000 left a comment

lukas-phaf Mar 4, 2024

lukas-phaf Mar 4, 2024

lukas-phaf Mar 6, 2024

Jeffrey-Vervoort-KNMI Mar 6, 2024

lukas-phaf Mar 6, 2024

Jeffrey-Vervoort-KNMI Mar 11, 2024

Teddy-1000 Mar 6, 2024

Teddy-1000 Mar 6, 2024

lukas-phaf Mar 6, 2024

lukas-phaf Mar 11, 2024

lukas-phaf Mar 11, 2024

		- name: Test client runs without errors
		run: DYNAMICTIME=false LOTIME=1000-01-01T00:00:00Z HITIME=9999-12-31T23:59:59Z docker compose run --rm client

Add parameter information to /collections and /locations endpoint #35

Add parameter information to /collections and /locations endpoint #35

Conversation

lukas-phaf commented Feb 13, 2024 • edited by Jeffrey-Vervoort-KNMI Loading

github-actions bot commented Feb 13, 2024 • edited Loading

API Unit Test Coverage Summary

Teddy-1000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukas-phaf commented Feb 13, 2024 •

edited by Jeffrey-Vervoort-KNMI

Loading

github-actions bot commented Feb 13, 2024 •

edited

Loading