-
Notifications
You must be signed in to change notification settings - Fork 11
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
As a user, I want to throw an WARNING when Time_Coordinates.start_time
does not match or come before Time_Coordinates.stop_time
#964
Comments
Do you mean if the stop time attribute appears in the label before the start time attribute? Or do you mean if the stop time is later in time than the start time? |
@jstone-psi per @matthewtiscareno comment, if it is the latter, this opens a bit of a can of worms. Per this comment:
The problem here is there is nowhere in the standard that requires the precision be the same, we can add that check, but we will also need to add a flag to disable it since some people may not care about that and/or past data sets didn't take that into account. We will need a set of test cases from the nodes in order to accurately test this (different time formats, different time precisions, invalid times, etc.). I could imagine we implement this and then get 5 bugs submitted because of corner cases we were unaware of. |
@matthewtiscareno @jordanpadams I did mean the latter. I understand this is a can of worms, and that there are a lot of corner cases that we would have to deal with. This would still be valuable to have in the general case, when everything matches up correctly. (I'd like to believe that this would happen most of the time). What would be the next step for this? Somewhere like DDWG or SWG, perhaps? |
@jstone-psi I will add this to our next SWG agenda. If you wouldn't mind speaking to it, and then we can ask the nodes for some of those corner cases. |
Understood. I will do my best to be there. |
Time_Coordinates.start_time
does not come before Time_Coordinates.stop_time
Time_Coordinates.start_time
does not come before Time_Coordinates.stop_time
Time_Coordinates.start_time
does not match or come before Time_Coordinates.stop_time
@jstone-psi FYI, I revised this ticket to be a new requirement vs. a bug fix |
Here are some notes that I prepared for the discussion: |
Time_Coordinates.start_time
does not match or come before Time_Coordinates.stop_time
Time_Coordinates.start_time
does not match or come before Time_Coordinates.stop_time
Here's one that incudes negative dates. It looks like this is from an earlier version of PDS that didn't support them, so the start date in the label is 0. All of the dates in this product are precise only to the year, so we won't be able to exercise comparing a negative date with different precision. https://pdssbn.astro.umd.edu/holdings/pds4-compil-comet:phys_char-v1.0/data/ |
@jstone-psi: I only spot-checked your examples, but I didn't find any where It seems to me that there should be an error (not a warning) if Your original proposal was simply to throw an error or a warning (I favor the former) if |
By the way, your two examples of nil |
@matthewtiscareno: The examples I posted were only to illustrate real-world labels where the precision was different (primarily for the sake of developing test cases). If I understood the issue from our archiving team correctly, then the instances where the I agree that situations where I would agree about the nil start_date_time or stop_date_time alone being an issue. I think that we've been ok with it when they are both nil for derived data. |
@jstone-psi: I suggest that such a test be implemented by simply reducing the more precise value to the precision of the less precise value. If they are equal (as in your example), then it's no problem. So is the conclusion that Validate does not test for start time later than stop time (and probably should), but that no actual examples of such a thing are currently known? |
I see from recent Software WG notes that you are also interested in discussing what to do more generally when there is a precision mismatch. That's a more nuanced topic, perhaps. Should it be a separate issue? |
|
Checked for duplicates
Yes - I've already checked
π§βπ¬ User Persona(s)
Data Archivist, Data Provider
πͺ Motivation
...so that I can ensure the start time comes before the stop time.
π Additional Details
From original bug ticket: from @jstone-psi:
Acceptance Criteria
Given a label with a start_time > stop_time
When I perform label validation
Then I expect validate to raise an WARNING
βοΈ Engineering Details
The text was updated successfully, but these errors were encountered: