-
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_EVENTDATE_FROM_YEARSTARTDAYOFYEARENDDAYOFYEAR #132
Comments
…or populate eventDate from year and start/end day of year. DESCRIPTION: Added DwCEventDQ.eventDateFromYearStartEndDay(), not yet annotated, along with unit test.
…G2 defintion. DESCRIPTION: added test for dwc:year=1999, dwc:startDayOfYear=123, dwc:endDayOfYear=125 therefore dwc:eventDate=1999-05-03/1999-05-05
I've added some potential text about the validity of 1999-123 (or 1999-123/1999-125) and proposed that these valid ISO date forms should not be used as the return value from this amendment. Not sure if this should be a SHOULD or MUST. |
As discussed elsewhere @chicoreus - this dependency was added because of three tests to populate eventDate that should be run in order of a priority. These dependencies, from memory, were to be temporary until we had a workflow and would be removed from the description. |
@ArthurChapman you are right, my goof. Must have though I was looking at the validation. We did indeed on a needed priority between the three amendments that can fill in an eventDate. |
Updating the implementation of this, I'm finding the specification (expected response) to be unclear: INTERNAL_PREREQUISITES_NOT_MET if dwc:eventDate was not EMPTY or dwc:year was EMPTY or both dwc:startDayOfYear and dwc:endDayOfYear were EMPTY or the values were not interpretable; AMENDED if dwc:eventDate was FILLED_IN from the values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear; otherwise NOT_AMENDED If the clause " or the values were not interpretable" applies to year, startDayOfYear, and endDayOf year, then I don't see a path to the NOT_AMENDED response.status. But, if that clause applies only to startDayOfYear and endDayOfYear, then a non-empty, but invalid value for dwc:year would result in a NOT_AMENDED response.status. Also, it isn't clear how to handle the case of startDayOfYear being empty and endDayOfYear containing a value, though it does seem logical to provide a single date if year and startDayOfYear have values but endDayOfYear doesn't. Rephrasing to fit the latest implementation in event_date_qc would give: INTERNAL_PREREQUISITES_NOT_MET if (1) dwc:eventDate was not EMPTY or (2) dwc:year was EMPTY or (3) both dwc:startDayOfYear and dwc:endDayOfYear were EMPTY or (4) if any not EMPTY values of dwc:year, dwc:startDayOfYear or dwc:endDayOfYear were not interpretable as numbers ; AMENDED if dwc:eventDate was FILLED_IN from the values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear; Something that would not propose a change if startDayOfYear was not specified would be: INTERNAL_PREREQUISITES_NOT_MET if (1) dwc:eventDate was not EMPTY or (2) dwc:year was EMPTY or (3) both dwc:startDayOfYear and dwc:endDayOfYear were EMPTY or (4) if any not EMPTY values of dwc:year, dwc:startDayOfYear or dwc:endDayOfYear were not interpretable as numbers ; AMENDED if (1) dwc:eventDate was FILLED_IN from the values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear or (2) if dwc:eventDate was FILLED_IN from the values in dwc:year, and dwc:startDayOfYear with dwc:endDayOfYear being EMPTY; otherwise NOT_AMENDED; |
@chicoreus That looks good to me, but why would you not AMEND is startDayOfYear was EMPTY and endDayOfYear was not? |
@tucotuco I propose that potential variant as year+startDayOfYear with no endDayOfYear feels like a logical choice that someone might make when mapping a collecting event date known to a single day onto year/startDayOfYear/endDayOfYear, the other logical choice being year+startDayOfYear, with endDayOfYear=startDayOfYear. Conversely year+endDayOfYear smells like some sort of error in mapping a date range of more than one day, so seeing that combination makes me more suspicious of it arising from an error condition of some sort. |
…t specifications. DESCRIPTION: Updating implementation of AMENDMENT_EVENTDATE_FROM_YEARSTARTDAYOFYEARENDDAY to fit current (2022-03-08) specification, adding and fixing unit tests to conform with current specifications. Adding notes to indicate notes in the issue and comments about clarity needed for implementation in the issue.
Hmm. I start squirming when there is subjectivity like this. There are other explanations we haven't covered as well that might better inform the choice. Two that come immediately to mind are the cases 1) some day on or before endDayOfYear=x and 2) some day on or after startDayOfYear=y. If we can't distinguish these cases from simply incomplete cases, then I think we shouldn't be amending either of them. |
@tucotuco Good. I like avoiding subjectivity. There is also the case where the eventDate spans more than one year, and potential variants in mapping that into start and end day of year. How about we change the specification from: INTERNAL_PREREQUISITES_NOT_MET if dwc:eventDate was not EMPTY or dwc:year was EMPTY or both dwc:startDayOfYear and dwc:endDayOfYear were EMPTY or the values were not interpretable; AMENDED if dwc:eventDate was FILLED_IN from the values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear; otherwise NOT_AMENDED To: INTERNAL_PREREQUISITES_NOT_MET if dwc:eventDate was not EMPTY or any of dwc:year, dwc:startDayOfYear, or dwc:endDayOfYear were EMPTY or any of the values in dwc:year, dwc:startDayOfYear, or dwc:endDayOfYear were not interpretable; AMENDED if dwc:eventDate was FILLED_IN from the values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear; if the value of dwc:endDayOfYear is less than the value of dwc:startDayOfYear, NOT_AMENDED I don't think this has a path that isn't covered, but if I've missed one, or to cover the potential, we could add an "otherwise" to the NOT_AMENDED clause. |
Good. I agree. |
Including otherwise, (which would cover any other cases individually valid values in year start/end day terms that aren't valid in combination e.g. endDayOfYear = 366 with year not a leap year): INTERNAL_PREREQUISITES_NOT_MET if dwc:eventDate was not EMPTY or any of dwc:year, dwc:startDayOfYear, or dwc:endDayOfYear were EMPTY or any of the values in dwc:year, dwc:startDayOfYear, or dwc:endDayOfYear were not independently interpretable; AMENDED if dwc:eventDate was FILLED_IN from the values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear; if the value of dwc:endDayOfYear is less than the value of dwc:startDayOfYear, or otherwise NOT_AMENDED |
Great @chicoreus and @tucotuco As a general principle, we should never AMEND something if there is doubt or where there is ambiguity. Comes into an earlier email where we have "X" as a date (month or day) - we have been inconsistent as to whether we AMEND it to "10" or not. If it ambiguous, we shouldn't AMEND. |
Just looking at the last version by @chicoreus I don't understand the last part "if the value of dwc:endDayOfYear is less than the value of dwc:startDayOfYear", Don't you mean "greater than"? |
@ArthurChapman "less than" should be correct. This is the case where it is NOT_AMENDED. |
Ah yes @tucotuco I missed the semicolon - looking at it as a comma. By bad |
Seems we have consensus, so I've updated the Expected Response. |
…t specifications. DESCRIPTION: Updating implementation of AMENDMENT_EVENTDATE_FROM_YEARSTARTDAYOFYEARENDDAY to fit updated (2022-03-10) specification, adding and fixing unit tests to conform with current specifications. Refining backing createEventDateFromParts utility method to explicitly handle year+startDayOfYear+endDayOfYear only, also changing this method to use LocalDateInterval to format the date string, making formatting of date ranges more consistent, updating unit tests to reflect this.
Suggest a mod to the Expected Response (note also incorrect ";"): AMENDED if dwc:eventDate was FILLED_IN from the values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear; if the value of dwc:endDayOfYear is less than the value of dwc:startDayOfYear, or otherwise NOT_AMENDED to AMENDED the value of dwc:eventDate from values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear if the value of dwc:endDayOfYear is less than the value of dwc:startDayOfYear, or otherwise NOT_AMENDED Not needing "FILLED_IN" seems good. |
OK but I would leave the "or" off "or otherwise, and add ";" for consistency AMENDED the value of dwc:eventDate from values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear if the value of dwc:endDayOfYear is less than the value of dwc:startDayOfYear; otherwise NOT_AMENDED |
Yep. Done. That was a typo |
@ArthurChapman - I think your logic is reverse? Don't we need AMENDED the value of dwc:eventDate from values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear if the value of dwc:startDayOfYear is less than the value of dwc:endDayOfYear? |
Of course - how did I get that so wrong? |
Changed "AMENDED" to "FILLED_IN" in accordance with discussions April 16. |
…MENDED when proposing changes to empty terms. Updated method, tests, and comments.
If you had ok values for dwc:year and dwc:startDayOfyear (cognisant of the Notes), the Expected Response would be INTERNAL_PREREQUISITES_NOT_MET. Is this correct? |
I think that is correct sometimes - assuming eventDate is EMPTY, the conditions still would not allow for an amendment to be made if the dwc:endDayOfYear was EMPTY or not interpretable. |
…t current (2023-06-09) test descriptions. Adding ProvidesVersion annotations. Removing now empty file stubs for checked methods. Removed deprecated wrapper for method. Addressed tdwg/bdq#132 AMENDMENT_EVENTDATE_FROM_YEARSTARTDAYOFYEARENDDAYOFYEAR
Added to Notes "This test is only for cases that fall within the one year (as given in dwc:year) and hence "dwc:startDayOfYear will always be less than dwc:endDayOfYear". [or do we just leave this as being obvious from the Expected Response." |
Edited Note to change "year and startDayOfYear" to "dwc:year and dwc:startDayOfYear" |
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" |
Changed Expected response from INTERNAL_PREREQUISITES_NOT_MET if dwc:eventDate was not EMPTY or any of dwc:year, dwc:startDayOfYear, or dwc:endDayOfYear were EMPTY or any of the values in dwc:year, dwc:startDayOfYear, or dwc:endDayOfYear were not independently interpretable; FILLED_IN the value of dwc:eventDate from values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear if the value of dwc:startDayOfYear is less than the value of dwc:endDayOfYear; otherwise NOT_AMENDED to INTERNAL_PREREQUISITES_NOT_MET if dwc:eventDate was not EMPTY or any of dwc:year, dwc:startDayOfYear, or dwc:endDayOfYear were EMPTY; FILLED_IN the value of dwc:eventDate from values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear if the values in each were independantly interpretable and if the value of dwc:startDayOfYear is less than the value of dwc:endDayOfYear; otherwise NOT_AMENDED |
This test currently (from the Notes) has dependencies - i.e. one should use dwc:verbatimEventDate, as a priority, before attempting to run this test. We have wanted all the tests to be stand-alone and thus, I think the Specification should be rewritten as follows INTERNAL_PREREQUISITES_NOT_MET if dwc:eventDate or dwc:verbatimEventData are bdq:NotEmpty or any of dwc:year, dwc:startDayOfYear, or dwc:endDayOfYear are bdq:Empty; FILLED_IN the value of dwc:eventDate from values in dwc:year, dwc:startDayOfYear and dwc:endDayOfYear if the values in each are independently interpretable and if the value of dwc:startDayOfYear is less than the value of dwc:endDayOfYear; otherwise NOT_AMENDED See also #93 where I have suggested a similar change. |
@ArthurChapman, as noted in #93, not a desirable change, it creates an
explicit interdependency and constrains how tests could be composed.
|
The text was updated successfully, but these errors were encountered: