fix: parse time function for nedlib
#139
Conversation
nedlibreader/time.go
Outdated
| return | ||
| } | ||
|
|
||
| err = fmt.Errorf("failed to parse string as time.Time: %s: %v", dateString, err) |
There was a problem hiding this comment.
| err = fmt.Errorf("failed to parse string as time.Time: %s: %v", dateString, err) | |
| err = fmt.Errorf("failed to parse string as time.Time: %s: %w", dateString, err) |
Wrap error to be able to use errors.Is or errors.As.
There was a problem hiding this comment.
This function will return the error message from the latest attempt which is a bit misleading considering we are trying different layouts.
Maybe we should return
time.ParseError{
Message: ": cannot parse as [list of formats?]"
}
considering:
// Error returns the string representation of a ParseError.
func (e *ParseError) Error() string {
if e.Message == "" {
return "parsing time " +
quote(e.Value) + " as " +
quote(e.Layout) + ": cannot parse " +
quote(e.ValueElem) + " as " +
quote(e.LayoutElem)
}
return "parsing time " +
quote(e.Value) + e.Message
}
There was a problem hiding this comment.
The error message is misleading, I agree. We should probably improve it, but right now the current implementation of this PR is better than what exists in main.
I added a fixup to fix the error handling in the invalid time format cases, so now we only check for the type, not the actual error message.
nedlibreader/time_test.go
Outdated
|
|
||
| func TestParseInvalidRFC1123(t *testing.T) { | ||
| dateString := "Tue, 05 Apr 2024 15:30:00" | ||
| expectedError := "failed to parse string as time.Time: Tue, 05 Apr 2024 15:30:00: parsing time \"Tue, 05 Apr 2024 15:30:00\" as \"Mon Jan _2 15:04:05 MST 2006\": cannot parse \", 05 Apr 2024 15:30:00\" as \" \"" |
There was a problem hiding this comment.
Better to compare error against time.ParseError using errors.As or errors.Is.
trym-b
force-pushed
the
fix/convert-time
branch
from
f2da2b6
to
c7a428a
Compare
|
Ready for next round @maeb |
trym-b
force-pushed
the
fix/convert-time
branch
from
c7a428a
to
3aacc01
Compare
|
After an IRL review with @maeb, we decided to return an additional argument from |
|
Ready for next round @maeb |
# Motivation `nedlib` data might contain a lot of different time formats. This commit adds a function to parse the time format used in `nedlib` data. # Future work Add parsing of custom time formats.
Motivation
nedlibdata might contain a lot of differenttime formats. This commit adds a function to parse the time format used in
nedlibdata.Future work
Add parsing of custom time formats.