Ord attributes #10
Ord attributes #10
Conversation
|
@mats-knmi @ritvje Please review it. This PR breaks the test, but we can fix it if we agree. |
| def_msg["properties"]["platform_name"] = "[" + nod + "]" | ||
|
|
||
| cc = str(nod)[:2].lower() | ||
| if def_msg["properties"]["naming_authority"] == "eu.eumetnet": |
There was a problem hiding this comment.
Is the idea that if the naming authority is already given in the message template, we don't change it? Or why is this if clause here?
There was a problem hiding this comment.
If it is a national product and the naming_authority is the default("eu.eumentnet"), the ingester updates naming_authority by country_naming_auth . Ingester skips this block when naming_authority was filled by NMS.
There was a problem hiding this comment.
This was a single site case, I added the composite case also, see: 1a94444
|
Sorry for the late response. To me this looks good, if you can update the test (files) in this PR as well then we can review in this same PR also what the impact is of this change to the example test files. |
…rdata-validator into ord-attributes
I updated test files. |
|
This pull request looks fine by me, other than the 2 small comments I had. I really like the way this pull request looks with the updated tests. Now I can see not just what the code changes are, but also what the impact is on the test files. This makes reviewing very easy. |
|
This looks good to me. |
Co-authored-by: mats-knmi <[email protected]>
Co-authored-by: mats-knmi <[email protected]>
3 participants
Added new ODIM ACDD attributes.