feat: support datetime formatting in output template - #554
Conversation
There was a problem hiding this comment.
Please add the corresponding unit test in file_name_utils_test.py to validate the new datetime formatting feature.
| @@ -56,7 +56,21 @@ def split_dims(self) -> t.List[str]: | |||
|
|
|||
| def formatted_output_path(self, splits: t.Dict[str, str]) -> str: | |||
There was a problem hiding this comment.
Please test that the output-template is handled correctly regardless of whether it is used with single quotes, double quotes, or no quotes at all!
| def formatted_output_path(self, splits: t.Dict[str, str]) -> str: | ||
| """Construct output file name with formatting applied""" | ||
| return self.unformatted_output_path().format(*self.template_folders, **splits) | ||
| import datetime |
There was a problem hiding this comment.
kindly move the import datetime statement to the very beginning of the file.
|
|
||
| try: | ||
| # First, substitute the positional args (if any) to avoid invalid f-string syntax like {0} | ||
| formatted_positionals = template.format(*self.template_folders, **{k: '{'+k+'}' for k in splits.keys()}) |
There was a problem hiding this comment.
template.format(...) doesn't support nested braces or function calls like {datetime.strptime(...), so it will raise a ValueError. When this happens, it falls into the except block and tries to call template.format() again, which will crash the pipeline. please fix this!
There was a problem hiding this comment.
please check and update the other functions in the pipeline that will not work as expected when they encounter these type of output-template!
|
I've pushed the fixes! The |
|
I've gone ahead and fixed the pipeline extraction logic! I updated |
|
I've just pushed an additional commit to resolve the linter |
- Move imports (ast, datetime, re) to module level in file_name_utils.py - Make formatted_output_path fallback safe: catch KeyError/ValueError/IndexError from str.format() and fall back to manual key substitution instead of crashing - Handle datetime templates in should_skip: return False early when template contains datetime expressions (wildcard glob matching not reliable) - Handle datetime templates in GribCopySplitter: catch format() errors and fall back to manual positional arg substitution - Add test for wildcard fallback with datetime templates
|
Addressed all review feedback:
All changes are scoped to |
2 participants
Closes #535
Summary
Adds datetime formatting support to the output template in
file_name_utils.py. Users can now use arbitrary f-string expressions withdatetimeobjects in file name patterns.Example
Before:
After:
Changes
weather_sp/splitter_pipeline/file_name_utils.py: supportdatetimeformatting in f-string evaluationweather_sp/splitter_pipeline/file_name_utils_test.py: add tests covering datetime format strings, fallback behavior, and edge casesVerification
Tests cover:
%Y,%m,%d,%H,%M,%S)