Default to UTC timezone #116
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
==========================================
- Coverage 95.41% 95.00% -0.42%
==========================================
Files 1 1
Lines 109 120 +11
==========================================
+ Hits 104 114 +10
- Misses 5 6 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yaml2ics.py
Outdated
|
|
||
| def event_from_yaml(event_yaml: dict, tz: datetime.tzinfo = None) -> ics.Event: | ||
| def event_from_yaml( | ||
| event_yaml: dict, tz: datetime.tzinfo = dateutil.tz.UTC |
There was a problem hiding this comment.
I think either change the default, or change the handling below, but not both.
There was a problem hiding this comment.
I feel it makes most sense to change the default here, and then let the event timezone overwrite. Or it maybe cleaner to drop the tz kwarg altogether and rely only on the yaml and have a conditional?
if "timezone" in d:
tz = gettz(d.pop("timezone"))
else:
tz = dateutil.tz.UTC
There was a problem hiding this comment.
Or, just break if the event_yaml does not have a timezone.
There was a problem hiding this comment.
OK, after #125 this becomes the crux of the current PR. We should document this in the README.
| # See RFC2445, 4.8.5 REcurrence Component Properties | ||
| # This function can be used to add a list of e.g. exception dates (EXDATE) or | ||
| # recurrence dates (RDATE) to a reoccurring event | ||
| def add_recurrence_property( |
There was a problem hiding this comment.
I think we make TZ required for now; this is for internal use anyway.
yaml2ics.py
Outdated
|
|
||
| def event_from_yaml(event_yaml: dict, tz: datetime.tzinfo = None) -> ics.Event: | ||
| def event_from_yaml( | ||
| event_yaml: dict, tz: datetime.tzinfo = dateutil.tz.UTC |
There was a problem hiding this comment.
OK, after #125 this becomes the crux of the current PR. We should document this in the README.
bsipocz
force-pushed
the
BUG_fix_default_tz
branch
from
7454841 to
765d473
Compare
|
I have rebased this on top of your PR, and will double check how it works with the SP calendars. Once it looks all sensible I'll come back to add some documentation and follow-up on any remaining comment (but I think the only remaining relevant one is about the docs). |
stefanv
changed the title
stefanv
added
type: Enhancement
|
This wasn't working yet, the UTC default is not propagated properly and we still had floating entries on the widget. |
|
OK I will make a follow-up PR. |
|
I just checked again, and I can confirm that UTC is propagated when no timezone is specified. |
2 participants
This made sense while doing the fixes for scientific-python/scientific-python.org#782, but also I separated it out as it may or may not be necessary.