niccokunzmann/python-recurring-ical-events

bug: X-WR-TIMEZONE not supported

niccokunzmann opened this issue ยท 11 comments

Describe the bug

It seems that several people encounter a behaviour which is unexpected: When X-WR-TIMEZONE is present, Google and other calendars show a events at a different time in different time zones.

To Reproduce

See Issues #53, #59, #61

Expected behavior

The expected behaviour of this module is to handle X-WR-TIMEZONE as these issues clearly show.

Version:

Additional context

Suggested implementation

This varies:

  1. This library can handle X-WR-TIMEZONE by default.
  2. A new library is reated to handle X-WR-TIMEZONE
  3. The user handles it, conclusion from #53, but not part of this Issue.

An untested implementation is discussed here.


We're using Polar.sh so you can upvote and help fund this issue. We receive the funding once the issue is completed & confirmed by you. Thank you in advance for helping prioritize & fund our work. Fund with Polar

For choosing way 2, I created this: https://github.com/niccokunzmann/x-wr-timezone
Collaboration welcome! We need examples. The idea is that people are not bound to this library but can still fix the issue.

My idea is to integrate the x-wr-timezone library once it works into this one by default.

@NicoHood @sirdan69 @asimonson1125 @martinm76 Notifying you as you helped in this regard. Also inviting you to collaborate!

For reference, a discussion in icalendar: collective/icalendar#343

I think the idea is to optionally depend on your new library and if it is available, it will normalize the input ical file? Or maybe you could just add the normalize to the example and its all good, no need to change a single line of this library?

Hm. My thought would be that Users of this library want it easy and not Test it out and run into a x-wr-timezone calendar later, untested. I would integrate it and use it by default, so that the meaning of x-wr-timezone is supported by default. This means also changing the major Version number. You would be able to opt out, though.

It only affects results of no timezone is used in the arguments.

What do you think of that? It's probably good to list pro and con of each Option.

Very interesting. I'll try it out on my work calendar soon and see if there is any difference from the modification I made inline when parsing it afterwards.

Pro (integrating):

  • There is no reason in not normalizing it. If you don't, the events are "broken"
  • You cannot do anything wrong, as the library implements it for you
  • No additional code in the user code, just a new dependency
  • If the calendar was already normalized, it would still be compatible

Con (not integrating)

  • If you (for some unknown reason) use the rest of the calendar not normalized, you have different timezones. So there must be a switch to disable that feature
  • It would work with old versions of this library. Not the best argument, but trust me, those setups exist in the wild.
  • Not backward compatible (major version bump)
  • New, hard dependency
  • You (very likely) need to normalize the calendar before the recurring filtering, if you also want to use it on different places. This means, that the library call to x-wr-timezone is useless
  • There are also other librarys that do not use x-wr-timezone as dependency, and there you need to normalize first anyways. It would be more freedome to the user which normalize app to use
  • Not all calendars use x-wr-timezone. Having this (hard) dependency would make it more complicated for those users, even if they do not need it. For example linux distributions now MUST package both librarys.

Suggestion:
Make a minimal change in your example and see how much the integration costs. I'd say it is an import, a function call and maybe an explanation comment. I'd go for the dependent solution first.

@martinm76 thanks for checking!

@NicoHood Thanks for the big list.

I created #77 so you can have a look.

For some reason, I still have problems with the tests - I think, having a few more example calendars would be useful. All tests run when I integrate x_wr_timezone - I did not expect that.

So: a good example would be a calendar using

  • X-WR-TIMEZONE that is Ameria/Asia, as far away from UTC as possible
  • one event with dtstart, dtend
  • one event with repetitions of which one was moved in time

If you can provide me with an ics file that does that, would be awesome! At the moment, I cannot create that myself - would take me hours and frozen fingers.

v1.0.0b supports X-WR-TIMEZONE. Please test it:) Thanks for your support!

I decided to include the small library. It does not really change much, only if you use at and between without timezone as argument. The tests indicated that no behavior that was tested needed to change.

Sounds good! I did a quick test on my work calendar and running my script on the original and the one piped though x-wr-tinezone, with ny astimezone calls intact, and the final result was identical. The calendars differed a fair bit, but most of it seemed to be down to the order of the fields. I only checked for a week, but it parsed the entire calendar without issue, so pretty sure it is doing exactly what it should ๐Ÿ˜€๐Ÿ‘๐Ÿป๐Ÿ™๐Ÿป

Hi Nico, I tried my test case against your latest changes around x-wr-timezone and it works where it used to crash. This was for the case where Confluence-created calendar contained only all-day events. Thanks for all your work on this issue!

thanks for your feedback! I will close this. It looks like it does about what it should.