-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TG2-AMENDMENT_GEODETICDATUM_ASSUMEDDEFAULT #102
Comments
Comment by Paul Morris (@chicoreus) migrated from spreadsheet: |
Parameter, as @ArthurChapman pointed out in #178 is which geodetic datum to use, not which vocabulary of geodetic datums to use.
Can only be interpreted as dwc:geodeticDatum is assumed to be "http://epsg.io" when not specified. This is clearly nonsensical, the default value should be WGS84, or the appropriate EPSG code, probably EPSG:4326. Parameter is definitely needed but should be:
or
With the notes specifying that the geodetic datum is taken from the http://epsg.io/ vocabulary.... |
@chicoreus This is the default sourceAuthority not the default value - so the value should be a value within that default source authority - whether it is ESPG:4326 (WGS84), EPSG:4674 (SIRGAS2000) or EPSG:7844 (GDA2020), etc. What you are testing for is a value in the field that is consistent with the a value in http://epsg.io/ - the same as you look for a country code in the ISO standard. On thinking about it - probably doesn't need to be Paramaterized and just accept that authority as THE authority. |
@ArthurChapman - you raise a nice distinction: A source (in this case again unique), but then the default value, of which there can be plenty. I can see therefore two scenarios for Parameterised?
|
@Tasilee This is the case for all tests (especially NOTSTANDARD and STANDARDIZED tests) that use an external Standard - ISO, DCMI, in this case EPSG, of any Vocabulary. The vocabulary, standard, etc. is the bdq:sourceAuthority and you are checking to see if the value in the record is a valid record in the bdq:sourceAuthority (in the case of Validations) or can be amended to conform with a value in the bdq:sourceAuthority (in the case of Amendments). In nearly all cases, there is only one sourceAuthority (except as @chicoreus mentions with Taxon names), so there is no choice of sourceAuthority needed, only the choice of a value from that sourceAuthority. Those few cases where there is a choice of sourceAuthority (taxon names) brings in both your 1) and 2) above. Thus, I agree with @chicoreus that we don't need as many Paramaterized tests as we have previously so tagged. Unless @tucotuco has justifications for them that we have not thought of. |
Phrasing of Amended text is unclear. Don't know how to interpret the comma:
Perhaps
|
I commented in @chicoreus email of September 1. We do need to standardize the phrasing. |
I just checked the editing history - back in August last year the wording had a "therefore" after the comma |
Sounds good to me. I've edited the ER as issues to discuss/fix are building. |
@ArthurChapman: I think this IS parameterised as it requires a default value for dwc:geodeticDatum? The Parameter isn't EPSG as a sourceAuthority, but which EPSG code is the default? The Example should have the assumption, e.g., "dwc:geodeticDatum is NULL and has been assumed EPSG:4326"? |
In line with recent discussions, there is ONE dq:sourceAuthority for the GDs, epsg.io, therefore that isn't a Parameter. In this case, why are we even including "specified source authority" other than than in References and possibly Notes to recommend use of EPSG codes? There is no need to access the source authority in the Expected response, simply to look up the Parameter value, e.g., EPSG:4326 as this is implementation dependent. There is no need for "EXTERNAL_PREREQUISITES_NOT_MET". I would also suggest amending the example to "dwc:geodeticDatum is NULL, so amended to dwc:geodetciDatum EPSG:4326". |
@Tasilee Parameterized as different users might want different defaults. This one is appropriate to have a parameter. |
I have been checking over additions to the Notes on Validations dependent on a Vocabulary adding "This test will fail if there are leading or trailing white space or non-printing characters." BUT, I cannot see that this AMENDMENT requires either the VOCABULARY tag or the new Notes text added. |
The Expected Response here does not conform to our usual phrasing. Could I suggest a change from If dwc:geodeticDatum is EMPTY, fill in the value of dwc:geodeticDatum with the value of bdq:defaultGeodeticDatum, report FILLED_IN and, if dwc:coordinateUncertaintyInMeters, dwc:decimalLatitude and dwc:decimalLongitude are NOT_EMPTY, amend the value of dwc:coordinateUncertaintyInMeters by adding the maximum datum shift between the specified bdq:defaultGeodeticDatum and any other datum at the provided dwc:decimalLatitude and dwc:decimalLongitude and instead report AMENDED; otherwise NOT_AMENDED. to INTERNAL_PREREQUISITES_NOT_MET if dwc:geodeticDatum is EMPTY; FILLED_IN the value of dwc:geodeticDatum using the value of bdq:defaultGeodeticDatum; AMEND the value of dwc:coordinateUncertaintyInMeters by adding the maximum datum shift between the specified bdq:defaultGeodeticDatum and any other datum at the provided dwc:decimalLatitude and dwc:decimalLongitude if dwc:coordinateUncertaintyInMeters, dwc:decimalLatitude and dwc:decimalLongitude are NOT_EMPTY; otherwise NOT_AMENDED. I realize the "report' bit is missing but we are assuming for other tests the activation of the phrase "FILLED-IN" or "AMENDED" or "NOT_AMENDED" etc apply as a report. So, if the FILLED-IN and AMENDED (or NOT_AMENDED) phrases are activated, they are reported? |
On Thu, 02 Feb 2023 14:33:24 -0800 Lee Belbin ***@***.***> wrote:
INTERNAL_PREREQUISITES_NOT_MET if dwc:geodeticDatum is EMPTY;
This clause doesn't belong. The purpose of this amendment is to supply a value for geodeticDatum when none is provided.
Probably this is why the current phrasing is unfamiliar, the case is unusual.
We likely want to keep the current phrasing, it did take some thinking through to get it refined into that state.
|
I think we want to keep the current phrasing. The recommended fix is not correct. |
Agree with @chicoreus. Suggestion by @Tasilee it is not correct that INTERNAL_PREREQUISITES_NOT_MET if the dwc:geodeticDatum is EMPTY - instead, you fill it in with the default. I looked at trying to reword it to make it a bit clearer, but can't do better than what we have. |
I note we have a "Source Authority is "epsg" [https://epsg.io]" but make no reference to it in the Expected Response. As such, should we delete that entry? I would think so. We have the Reference to epsg. |
I think so - an earlier iteration of the ER must have cited it and it wasn't removed with altered wording. |
I have amended the Source Authority accordingly |
Restructured Parameter(s) and Source authority |
Changed EPSG: to epsg: in the Source Authority and in the Examples Changed "TG2-AMENDMENT_COORDINATES_CONVERTED" to AMENDMENT_COORDINATES_CONVERTED (620749b9-7d9c-4890-97d2-be3d1cde6da8) in the Notes |
Changed epsg: to EPSG: in the Source Authority and in the Examples |
Updated note to remove internal GitHub Reference. |
Amended Source Authority to align with @chicoreus syntax From bdq:defaultGeodeticDatum default="EPSG:4326" to bdq:defaultGeodeticDatum default="EPSG:4326" {[https://epsg.org/]} but do we need an API endpoint? |
On advice from EPSG - changed in References: "https://www.epsg.org/" to "https://epsg.org/" Note it was already correct in Source Authority |
Corrected Source Authority entry from bdq:defaultGeodeticDatumt = "EPSG:4326" {[https://epsg.org/crs_4326/WGS-84.html]} to bdq:defaultGeodeticDatum = "EPSG:4326" {[https://epsg.org/crs_4326/WGS-84.html]} |
Removed reference to bdq:sourceAuthority from Source Authority |
Corrected typo in information elements s/dwc:decimatitude/dwc:decimalLatitude/ |
…-06-28) specifications. Addressing tdwg/bdq#102 AMENDMENT_GEODETICDATUM_ASSUMEDDEFAULT adding implementation of 5 degree cell lookup table for coordinate uncertainty addition from unknown datum to WGS84 derived from VertNet georeferencing calculator. Added missing parameters to amendmentGeodeticdatumAssumeddefault(), a default implementation, and minimal unit tests.
After @chicoreus email July 25 2023, I changed the Positive example from FILLED_IN to AMENDED. |
Don't we use AMENDED where a value has been altered, and FILLED_IN where the field was empty and we added something - see definitions in #152. I believe for consistency this should be FILLED_IN - not sure I saw @chicoreus email. |
@ArthurChapman This one is more complex than usual, and almost calls for a richer response structure. When only an empty geodeticDatum is filled in, then response.status is FILLED_IN, but if a value is present in coordinateUncertaintyInMeters and an empty geodeticDatum is filled in then the coordinateUncertaintyInMeters is ammended to add the uncertainty between any datum and the specified default at the specified latitude/longitude, and the response.status becomes AMENDED. |
Thanks @chicoreus - understood. |
Splitting bdqffdq:Information Elements into "Information Elements ActedUpon" and "Information Elements Consulted". Also changed "Field" to "TestField", "Output Type" to "TestType" and updated "Specification Last Updated" |
The text was updated successfully, but these errors were encountered: