Implement kalendar module #189
Conversation
caldwellst
added
the
enhancement
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## develop #189 +/- ##
===========================================
- Coverage 90.77% 90.38% -0.39%
===========================================
Files 19 19
Lines 1311 1300 -11
===========================================
- Hits 1190 1175 -15
- Misses 121 125 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Hey Seth, this is cool, I didn't know about @overload! I was reading a bit and I think the preferred way to do this in Python (which maybe could be applied to some other methods, also in Kalendar -- sorry I should have thought of this earlier!) is to use dispatching, see for example this article. Do you think it could work for this case?
| @@ -102,15 +98,19 @@ def __init__( | |||
| self._data_variable_suffix = data_variable_suffix | |||
|
|
|||
| # set dates for data download and processing | |||
| self._start_date = get_dekadal_date( | |||
| input_date=start_date, default_date=_EARLIEST_DATE | |||
| self._start_date = get_kalendar_date( | |||
There was a problem hiding this comment.
Why didn't you keep get_dekadal_date here?
There was a problem hiding this comment.
hahaha, because I am a dingus. I didn't have get_dekadal_date() at first.
| ) -> Tuple[int, int]: | ||
| """Calculate dekadal date from general input. | ||
| @overload | ||
| def get_kalendar_date( |
There was a problem hiding this comment.
Why are these two overloaded functions needed - looks like the last one takes care of everything?
There was a problem hiding this comment.
It was just for the typing really, but I think I was slightly misusing it. Will revisit after I do some learning on dispatching.
Implements #106.
I brought in the
kalendarlibrary into ourdates.pymodule. I think we still need our own module to allow for the broader implementation of the user inputted ISO dates/strings and wider work, but has made the implementation much simpler.This has all been used on the backend for NDVI. However, I made the decision NOT to include the dekad class on the end output. I think that would make it confusing for end users if they potentially saw classes they weren't familiar with or had no desire to use, so rather kept the format of output files having year, dekad, and date columns.
Look forward to your review and thoughts!