Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time zone conversion support #161

Closed
perette opened this issue Dec 25, 2018 · 4 comments · Fixed by #294
Closed

Time zone conversion support #161

perette opened this issue Dec 25, 2018 · 4 comments · Fixed by #294
Milestone

Comments

@perette
Copy link
Contributor

perette commented Dec 25, 2018

The project I'm working on deals with timezones, so I've ended up writing code that convert events to a timezone by changing the timezones on the start & end times; standard and all-day events get handled differently, since a time-based event changes displayed times as it moves timezones, but all-day events always run midnight to midnight in whatever timezone they are in, based on their dates.

I think these might be useful to the library. I was thinking following the datetime manner, so Event.astimezone(tz=zone) and, to do everything in a calendar, Calendar.astimezone(tz=zone).

If these would be utilitarian, I'd be glad to add them.

@C4ptainCrunch
Copy link
Member

I guess we should first your PR #159 but afterwards, if it's still needed yes it would be nice :)

@C4ptainCrunch
Copy link
Member

Also, maybe i made a mistake to represent dates in all-day events as datetimes and not just date. Maybe we could also change that. (But then, event.begin could have 2 different types, that is not pretty (Or we could make another class, AllDayEvent but that might not be prettier either))
What do you think ? @perette @N-Coder

@N-Coder
Copy link
Member

N-Coder commented Jun 14, 2019

Phew, though question! I also already put some thought into this, also wanting to split off the make_all_day stuff into its own event class at first.

In general, having two different datatypes for begin and end would probably make using the library a lot harder, because you need to use an if-check everytime you want to access the time, so defaulting to midnight seems to be a good approach. Additionally, using two seperate classes would make conversion problematic if the event is contained in some other datastructure (i.e. a Calendar). As the current API is completely focussed on making changes inplace instead of working on immutable datastructures (which is perfectly fine in our case), this would break the overall semantics and make use cases like toggling all-day events in an UI pretty hard to realize (as long as there is no Calendar.replaceEvent or similar).

So probably staying with the date+time+timezone arrow object for all Events without an additional class would be the best approach. But as time+timezone don't make sense for all-day events and can't be represented by the backing file format, I'd make sure that those can't be set for all-day events and always have the same default value, e.g. midnight and no timezone information set (this can also be added to our list of generic assumptions using something like the following - which is untested as I have don't have the source code at hand right now).

self.assertEqual(precission_day.begin.time, time(0, 0))
self.assertEqual(precission_day.begin.tzinfo, None)
self.assertEqual(precission_day.begin, arrow.get(precission_day.begin.date)) # make sure the Arrow objects contains nothing more than the date information

This raises the question on how the begin/end setters should react when receiving an arrow object that contains this information for an all-day event. Should this information be silently dropped, should we use the same conversion process as in make_all_day, should we throw an exception, ...? Depending on this behaviour, Event.astimezone can probably be implemented as simple delegator to setting begin and end correspondingly.

@N-Coder
Copy link
Member

N-Coder commented Aug 29, 2021

This is now implemented for all Events and ToDos in a Calendar in #294 using

calendar.normalize(
    Timezone.from_tzid("America/Toronto"), # target timezone
    normalize_floating=NormalizationAction.REPLACE, # replace tz of floating events, use IGNORE to skip those
    normalize_with_tz=NormalizationAction.CONVERT) # convert tz of events that know their tz

See tests/normalization.py for a more complete example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants