Closed BSchilperoort closed 2 years ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
Kudos, SonarCloud Quality Gate passed!
Thanks for addressing. Indeed, I think it's fine to merge this and restructure in another PR. One more comment that I forgot to write before: with the addition of a weekly and monthly calendar, I think the adventcalendar feels a bit awkward now. Shall we consider changing it to daily calendar? And/or shouldn't they be called NDayCalendar, NWeekCalendar, NMonthCalendar?
Hm. An alternative could also to have the users interface with a single calendar object, e.g. AdventCalendar (or CountdownCalendar or any other suitable name). Where the following would all be valid inputs;
daycalendar = AdventCalendar(anchor=(31, 12), freq='4d')
weekcalendar = AdventCalendar(anchor=40, freq='2W')
monthcalendar = AdventCalendar(anchor='Jan', freq='1M')
This PR refactors the current calendars into a BaseCalendar class, where most methods are implemented, and subclasses like
AdventCalendar
orMonthlyCalendar
.As an example I implemented two extra calendars;
MonthlyCalendar
: where a user inputs an anchor month, and a freq in the form1M
WeeklyCalendar
: where a user inputs an anchor week (as in, week of year), and a freq in the form1W
Additionally, the implementation of resample has been moved to a separate module, as
s2spy.time
had gotten too large. Resample now also works as discussed in #50List of changes:
.show()
or.get_intervals()
.s2spy.time
ands2spy._base_calendar
have been moved tos2spy.utils
.Demo of functionality
Still to do:
Add check to see if the data can fully cover the calendar, and warn the user if it cannotOut of scope for current PR see #67Ensure that if the data is insufficient to cover the calendar, NaNs will be used for the empty intervalsWorks for xarray data, not for pandas data. Out of scope for this PR. See #67Also:
This PR: Closes #33 Closes #41 Closes #50