Refactor deployment time extraction and error handling#548
Refactor deployment time extraction and error handling#548
Conversation
used regular expression to parse deployment dates
There was a problem hiding this comment.
Please add unit test -- should handle the following:
{deployment_name}-{ISO8601LIKE} (base case)
{deployment_name}-{ISO8601LIKE}Z UTC TZ specified, possibly handle other TZ, although I rarely see this in practice
{deployment_name}-{ISO8601LIKE}-{extra_string} Often extra_string is "delayed" for delayed mode data, pursuant to #355
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Add functions to validate and normalize timestamps.
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
| # Build ISO string and let dateutil validate calendar correctness (e.g., April 31) | ||
| iso = f"{y:04d}-{mo:02d}-{d:02d}T{hh:02d}:{mm:02d}:{ss:02d}Z" | ||
| try: | ||
| dt = isoparse(iso) |
There was a problem hiding this comment.
isoparse should handle most of these validation cases:
>>> from dateutil.parser import isoparse
>>> isoparse("9999-99-99T27:68:68")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/local/lib/python3.8/site-packages/dateutil/parser/isoparser.py", line 37, in func
return f(self, str_in, *args, **kwargs)
File "/usr/local/lib/python3.8/site-packages/dateutil/parser/isoparser.py", line 146, in isoparse
return datetime(*components)
ValueError: month must be in 1..12
I don't see an import statement for dateutil.parsers.isoparse so I think this will result in a runtime error when this line is run.
There was a problem hiding this comment.
that is right I need to add that
| iso = f"{y:04d}-{mo:02d}-{d:02d}T{hh:02d}:{mm:02d}:{ss:02d}Z" | ||
| try: | ||
| dt = isoparse(iso) | ||
| except Exception as exc: |
There was a problem hiding this comment.
Don't catch raw exceptions, change this to catch ValueError and log the exception, proceeding with logic for failure conditions.
Handle parsing errors for ISO date strings by logging exceptions and appending error messages to the report list.
benjwadams
left a comment
benjwadams
left a comment
There was a problem hiding this comment.
Per earlier comment I intended as a code review, please add unit tests for at least the following cases:
{deployment_name}-{ISO8601LIKE} (base case)
{deployment_name}-{ISO8601LIKE}Z UTC TZ specified, possibly handle other TZ, although I rarely see this in practice
{deployment_name}-{ISO8601LIKE}-{extra_string} Often extra_string is "delayed" for delayed mode data, pursuant to https://github.com/ioos/glider-dac/issues/355
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
^^ I guess this is a review, I was confused by the comment portion. |
added <att name="dac_qc_comment">...</att> in the <addAttributes> block in ERDDAP XML chunk to overwrite the value from the NetCDF files in the ERDDAP aggregated dataset.
Fix dac_qc_comment attribute to aggregated dataset
|
Commit 5ec220c says "Fix trailing whitespace and end-of-file issues", but it appears to have much more than just formatting logic. Could you please rebase and split out some of these changes into separate commits, assuming you don't want to squash the formatting changes into the original commit upon rebase. |
2 participants
used regular expression to parse deployment dates