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

Recurring and floating events #97

Merged
merged 11 commits into from
Dec 18, 2022

Conversation

eigenmannmartin
Copy link
Member

Ical format does not have a 'default' timezone for the calendar but instead 'floating' events. Events can have a date, datetime or a datetime with timezone information as start and end. This is pr implements this functionality. This pr also fixes some recurring issues with custom defined timezones where events will be unfolded to the wrong time it the recurrence passes the timezone border.

The api now alloes three new parameters:

  • tzinfo force output to be converted to this timezone
  • sort sorts the events according to its start date.
  • strict if set to true will output dates, datetimes and datetimes with timezones as specified in the ical. If set to false (or unset) will behave the same as before for backward comparability.

Missing right now is the documentation but I wanted to have someone else to see this first and verify that this version works as expected.

@codecov
Copy link

codecov bot commented Dec 27, 2021

Codecov Report

Merging #97 (53817cc) into master (2f82ffe) will increase coverage by 1.14%.
The diff coverage is 92.56%.

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   90.07%   91.22%   +1.14%     
==========================================
  Files           4        4              
  Lines         383      376       -7     
  Branches       98       98              
==========================================
- Hits          345      343       -2     
+ Misses         17       14       -3     
+ Partials       21       19       -2     
Impacted Files Coverage Δ
icalevents/icalparser.py 92.55% <92.50%> (+1.54%) ⬆️
icalevents/icalevents.py 90.19% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mbafford
Copy link

mbafford commented Mar 30, 2022

@eigenmannmartin This PR fixes the issues I noticed with #110 (same as #102, as you pointed out). However, with this code my test test_multi_exdate_same_line from #109 fails to include in the final 2022-04-29T11:00-0400 entry from the RRULE:

RRULE:FREQ=WEEKLY;UNTIL=20220429T150000Z;INTERVAL=1;BYDAY=FR;WKST=MO
EXDATE;TZID=Eastern Standard Time:20220318T110000,20220401T110000,20220408T
 110000
UID:040000008200R00074P5O7101N82R00800000000R0Q793689428Q801000000000000000
 010000000QOPN4SS024R0264S9P0Q7OOQQ16PN399
SUMMARY:Recurring With Exclusions
DTSTART;TZID=Eastern Standard Time:20220311T110000
DTEND;TZID=Eastern Standard Time:20220311T113000

Apple Calendar is pulling in the entry for 2022-04-29 11:00 US/Eastern, but your changes are not. Does your code have a < when it should be <=? (inclusive end time for the RRULE UNTIL?), or some timezone issue?


Stepping through the debugger, looks like the timezone conversion is wrong/missing:

From rrule.py:

                for i in dayset[start:end]:
                    if i is not None:
                        date = datetime.date.fromordinal(ii.yearordinal + i)
                        for time in timeset:
                            res = datetime.datetime.combine(date, time)
                            if until and res > until: # <----  **** this line
                                self._len = total 
                                return

At the indicated line above:

until == '2022-04-29 10:00:00+00:00'
res == '2022-04-29 11:00:00-05:00'

I don't have a good grasp on where the code needs to be updated to fix this test, however. Hopefully it jumps out at you.

@eigenmannmartin eigenmannmartin force-pushed the recurring-and-floating-events branch 3 times, most recently from a11bb1d to fa2c69e Compare December 18, 2022 13:35
@eigenmannmartin
Copy link
Member Author

@mbafford I have shamelessly included your test in this mr. Great catch!

@eigenmannmartin eigenmannmartin merged commit c0c9c46 into jazzband:master Dec 18, 2022
capuanob pushed a commit to ennamarie19/icalevents that referenced this pull request Mar 6, 2023
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 this pull request may close these issues.

2 participants