-
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_MINELEVATIONMAXELEVATION_FROM_VERBATIM #68
Comments
Comment by Paul Morris (@chicoreus) migrated from spreadsheet: |
Shouldn’t we add that dwc:minimumElevationInMeters and dwc:maximumElevationInMeters are both EMPTY as a prerequisite? |
As defined, whether or not elevation terms will not be filled in if one is populated is ambiguous for implementors. Compare with explicit language in #55 , where both terms must be populated to avoid amendment, but one empty term can be amended. This is also one of a number of amendments where terms with existing values should probably return NOT_CHANGED instead of INTERNAL_PREREQUISITES_NOT_MET, a general discussion of these can be filled in from another term amendments is needed. |
I think your logic is correct - seems only difference with #55 was to add "and/or" instead of "or". Also your comment re INTERNAL_PREREQUISITES_NOT_MET versus NO_CHANGED also makes sense |
Thanks @chicoreus and @ArthurChapman. I buy the NOT CHANGED argument. See @chicoreus email of September 1 for a summary of the issues involved. |
In looking at the test data, we may have an anomaly with the Expected Response. If dwc:verbatimElevation has say "min elevation 10m", dwc:minimumElevation="" and dwc:maximumElevation="100", then the AMENDMENT will not be applied with INTERNAL PREREQUISITES_NOT_MET. Maybe something like INTERNAL_PREREQUISITES_NOT_MET if dwc:verbatimElevation is EMPTY or the value(s) is not unambiguously interpretable or the target of the value(s) of dwc:verbatimElevation (dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters) are not EMPTY; AMENDED if the values of dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters were unambiguously interpreted from dwc:verbatimElevation; otherwise NOT_AMENDED |
That makes sense to me. NOTE that the same would apply for the equivalent Depth test |
I believe that is the correct outcome and I would not change the Expected Respone. If either of min or max elevation is already filled in, don't amend. Doing so can create an inconsistency. For example, if dwc:verbatimElevation has "sea level", dwc:minimumElevationInMeters="10" and dwc:maximumElevationInMeters="". What should be the outcome? Asking more of implementers is too much, I believe. |
@tucotuco I concur. General principle, when a non-verbatim field contains a value, the amendments that fill in from verbatim shouldn't change that value, they should only fill in an empty value. Thinking back, this may have been one of our reasons for thinking of FILLED_IN as at the same level as AMENDED. |
Certainly what @tucotuco says is simpler and less likely to cause problems. |
OK, consensus. I'll ensure the test data aligns. |
Changed "AMENDED" to "FILLED_IN" in accordance with discussions April 16. |
This Expected Response is a classic example where there is redundancy between "INTERNAL_PREREQUISITES_NOT_MET" and "NOT AMENDED". Is the application of "NOT AMENDED" even possible? INTERNAL_PREREQUISITES_NOT_MET if dwc:verbatimElevation is EMPTY or the value is not unambiguously interpretable or dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters are not EMPTY; FILLED_IN the values of dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters if they could be unambiguously interpreted from dwc:verbatimElevation; otherwise NOT_AMENDED |
On Sat, 28 Jan 2023 14:59:45 -0800 Lee Belbin ***@***.***> wrote:
dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters are
not EMPTY;
The not empty clause is the problematic clause, if the target terms for the amendment to populate are not empty, then the amendment should end up in NOT_AMENDED.
|
I thought I agreed with @chicoreus here, but on a closer look, I think what we have is correct. If the AMENDMENT stands alone as we previously decided, then you don't want to run the test if there is a value in both dwc:minimumElevationInMeters and dwc:maximumElevationInMeters. Thus, as I see it - the requirements to run the AMENDMENT do not exist, which would imply that the INTERAL_PREREQUISITES to run the test have not been met. If you go with the interpretation of @chicoreus, then you'd need to reword the AMEND part to make sure an amendment is not made to an already existing value. |
Unless I am going mad (always possible), I think you both have missed the point. Yes, I agree that if there are values in dwc:minimumelevation or dwc:maximumelevation, they should never be overwritten. My point is the repetition of the "unambiguously interpretable" in the INTERNAL_PREREQUISITES_NOT_MET and the equivalent "unambiguously interpreted from" in the FILLED_IN section. If the text/values in dwc:verbatimElevation are not unambiguously interpretable, this will trigger INTERNAL_PREREQUISITES_NOT_MET when I think it should trigger the NOT_AMENDED! So, I am suggesting that we remove occurrences of "unambiguous ..." in the INTERNAL_PREREQUISITES_NOT_MET sections of Expected Responses. Here is another example from #71: "EXTERNAL_PREREQUISITES_NOT_MET if the bdq:sourceAuthority is not available; INTERNAL_PREREQUISITES_NOT_MET if dwc:taxonID is EMPTY, the value of dwc:taxonID is ambiguous or dwc:scientificName was not EMPTY; FILLED_IN the value of dwc:scientificName if the value of dwc:taxonID could be unambiguously interpreted as a value in bdq:sourceAuthority; otherwise NOT_AMENDED" I would suggest that "the value of dwc:taxonID is ambiguous" should be removed from INTERNAL_PREREQUISITES_NOT_MET because it is already covered by the statement "the value of dwc:taxonID could be unambiguously interpreted..." in the "FILLED_IN" section. |
@Tasilee Your conclusion seems logical to me. If "the value of dwc:taxonID is ambiguous", that should simply result in NOT_AMENDED. |
Perhaps a way around this is to partly parallel #56 (see my comments there) and make a difference between "invalid" and " ambiguous" INTERNAL_PREREQUISITES_NOT_MET if dwc:verbatimElevation is EMPTY or the value is invalid or dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters are not EMPTY; FILLED_IN the values of dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters if they could be unambiguously interpreted from dwc:verbatimElevation; otherwise NOT_AMENDED |
In this case, we would likely expect to interpret a text string in dwc:verbatimElevation so @chicoreus 's comment about TYPE and INTERNAL_PREREQUISITES_NOT_MET is relevant: We wouldn't trigger the INTERNAL_PREREQUISITES_NOT_MET so "invalid" doesn't seem applicable. I would have the Expected Response as INTERNAL_PREREQUISITES_NOT_MET if dwc:verbatimElevation is EMPTY or dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters are not EMPTY; FILLED_IN the values of dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters if they could be unambiguously interpreted from dwc:verbatimElevation; otherwise NOT_AMENDED |
I hate to be thinking this... What about a case such as "<=200". There one of the two could be filled in and the other could not? That makes this amendment complex and suggests that two separate amendments might be more tenable, one for minimum elevation and one for maximum elevation. |
@tucotuco Wouldn't this just be a case where you would only fill in dwc:maximumElevationInMeters (i.e. =200). and you'd leave dwc:minimumElevationInMeters empty because all you know about the minimum elevation is that it is less that 200. This (as in the Note) is a range, but you don't know the lower end of the range. |
@ArthurChapman Yes. I feel stupid, but much better. I was tricked by " if they could be unambiguously interpreted". I suggest: "INTERNAL_PREREQUISITES_NOT_MET if dwc:verbatimElevation is EMPTY or dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters are not EMPTY; FILLED_IN the values of dwc:minimumElevationInMeters and/or dwc:maximumElevationInMeters that could be unambiguously interpreted from dwc:verbatimElevation; otherwise NOT_AMENDED" |
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" |
Needs work on line with #55 |
Following suggestions in #55 INTERNAL_PREREQUISITES_NOT_MET if dwc:minimumElevationInMeters and dwc:maximumElevationInMeters are not EMPTY and either dwc:verbatimElevation is EMPTY or the value is not unambiguously interpretable; FILLED_IN the value of dwc:minimumElevationInMeters and dwc:maximumElevationInMeters if they are EMPTY and could be unambiguously determined from dwc:verbatimElevation; otherwise NOT_AMENDED. |
Expected Response and Notes updated following discussions under #55 and Specification Last Updated, updated. |
…TION_FROM_VERBATIM and improving implementation of tdwg/bdq#55 AMENDMENT_MINDEPTH-MAXDEPTH_FROM_VERBATIM. Implementations up to date with current specifications, along with unit tests.
Similarly to #55, I suggest we change the Expected Response INTERNAL_PREREQUISITES_NOT_MET if dwc:minimumElevationInMeters and dwc:maximumElevationInMeters are EMPTY and either dwc:verbatimElevation is EMPTY or the value is not unambiguously interpretable; FILLED_IN the values of dwc:minimumElevationInMeters and dwc:maximumElevationInMeters if they are EMPTY and could be unambiguously interpreted from dwc:verbatimElevation; otherwise NOT_AMENDED to INTERNAL_PREREQUISITES_NOT_MET if dwc:verbatimElevation is EMPTY or the value is not unambiguously interpretable; FILLED_IN the values of dwc:minimumElevationInMeters and dwc:maximumElevationInMeters if they are EMPTY and could be unambiguously interpreted from dwc:verbatimElevation; otherwise NOT_AMENDED ? |
I've changed the Expected Response as above, and updated the Specification Last Updated. I have also added two more records to the Test Data, one for this test (and a matching one for #55) for the edge case dwc:verbatimDepth="100 m" |
Changed to and updated Specification last updated. |
Changed "or" to "and" in the INTERNAL_PREREQUISITES_NOT_MET to allow for either latitude or longitude to be filled in independantly updated Specification Last Updated |
Removing dashes to make TERM_ACTION consistent. |
…s for additional tests: tdwg/bdq#62 tdwg/bdq#68 cleaining up comments, removing an extraneous @param.
The text was updated successfully, but these errors were encountered: