Feature request: calculation of duration and/or dtend/due

[tobixen]

According to the RFC, an icalendar VEVENT can have the property DURATION or the property DTEND, but it's not allowed to have both set. I believe the intention is that an event with DTEND set one hour after the DTSTART is to be considered equivalent with an event with DURATION set to PT30M.

The same applies to tasks, they can have a DURATION or a DUE, but never both. I suppose the intention there also is that a task with DTSTART and DURATION set should be considered equivalent with a task with DSTART and DUE set (though the tasks are a bit more muddy in my opinion - say there is three hours of paperwork that needs to be done before the end of the year, in my opinion it would make sense to set DUE to 2023-01-01, DURATION to three hours ... but one would probably want to start with it in early December, at least some days before the new years eve).

In the caldav library I now have methods get_duration() , get_due(), set_due() and set_duration(), which will attempt to do the right thing - like, get_duration() will yield DURATION if it's set, otherwise DTEND-DTSTART. It makes sense generalize it or also add get_dtend and set_dtend. Anyway ... this is logic that I need, but I feel it does not belong in the caldav library. If anything, it belongs in the icalendar library - or perhaps a separate new package like icalendar_util. (or perhaps there already exists some package like that?)

I will gladly contribute with a pull request on this one, but not without feedback that it's wanted and appropriate to put it into the icalendar library.

[niccokunzmann]

I like it. Let's make it happen. Deduplicate code, TDD. I have the same in recurring-ical-events. Please also note the PERIOD value type. It is good to get this straight for everyone. If we can pass an rdate value, replacement start or end etc to this, that would be much appreciated later. I have PERIOD value type support still open. It would be nice to use DUE or DTEND or PERIOD for the setter depending on what is used or choose the end property all together. I mean: while there are different wishes, making it simpler now and better later is probably good. I am happy to support.

[tobixen]

The implementation details and the API should be decided upon.

There is some common logic that will apply both to cal.Todo and cal.Event. Those inherits the cal.Component class. The latter has 7 other child classes that don't have a DURATION (as far as I know ... don't have time to read up the RFC again on this one right now).

As for the implementation, I see some alternatives:

• Creating independent methods in Todo and Event classes. There will be some duplicated logic. It's not a lot of logic, but still ... I hate code duplication, so I don't like it :-)
• Creating public methods in the Component class ... which will raise an error if the component doesn't support DURATION. I think it's not a good solution as we're "polluting" the namespace for many classes that don't need it.
• Creating public methods in Event and Todo, but putting common logic into "private" methods in the Component class. I think I favor this approach.
• Make a new subclass between Component and Todo/Event.

[tobixen]

(came to think, I read some books about object-oriented design around 30 years ago, from that perspective the cleanest approach would be to have a separate subclass between Component and Todo / Event ...)

[tobixen]

Any opinions on the implementation details?

(I also touched upon this in a totally unrelated comment in the recurring-ical-event project, the component types VEVENT, VTODO and VJOURNAL is quite much related, can have almost all the same properties. Both VTODO and VJOURNAL is "event-like", a task is basically an event with flexible start and end-time, and a journal is basically an event in the rearview mirror. From that perspective a common superclass would be appropriate. I should check the RFC to see if there is any naming convension there - otherwise, I would suggest something like "EventLike") ...

[niccokunzmann]

For me, superclass or shared function + polymorphy and other approaches are up to the implementing person/implementation detail. It would be nice to have a specification and tests. I wonder about cases: - start - end -start, end -start, duration -start,end,duration -what is with period value type -how does it relate to repetitions? Would that be rather interesting for the return value of recurring-ical-events or also for general component with repetitions? Would it return a value or raise an error if there is unclarity?

[angatha]

I think introducing a more complex class hierarchy is not the way to go, if it does not yield benefits. Currently it takes not much effort to check the implementation for completenes aginast the RFC because the class definition of e.g. Event lists all properties.

For the case of duration/end getters and setters, I would add a fifth option:

Use pythons multiple inheritance feature to add the methods just to cal.Todo and cal.Event by making them inherit from a yet to be named class implementing the getters and setters:

class Event(ClassImplementingMethids, Component):
    ...

[tobixen]

This one fell under the radar, sorry:

It would be nice to have a specification and tests.

It's often a nice practice to write the test code before the actual code ... but anyway, here some back-of-the-envelope specification/documentation/pseudocode - disregarding the Todo class as for now:

class Event

    (...)

    @property
    def duration(self: Event) ->  datetime.timedelta
    """
    If 'DURATION' in self, return self['DURATION'].

    If 'DTSTART' and 'DTEND' in self, return self['DTEND'].dt-self['DTSTART'].dt

    Otherwise, return None
    """
    (...)

    @duration.setter
    def set_duration(
        self: Event, 
        duration: datetime.timedelta, 
        movable_attr: Literal['DTSTART', 'DTEND']='DTEND') -> None:
    """
    If movable_attr is DTEND and DTSTART is set , adjust DTEND 

    (DTSTART should always be set for an Event)

    If movable_attr is DTSTART and DTEND is set, adjust DTSTART

    After adjustment, self['DTEND']-self['DTSTART'] == duration.

    Otherwise, set self['DURATION'] = duration
    """

    @property
    def dtend(self: Event) -> datetime
    """"
    If 'DTEND' in self, return self['DTEND'].dt
    If 'DURATION' and 'DTSTART' in self, return DTSTART + DURATION
    Otherwise, return None
    """"

    @dtend.setter(self: Event, dtend: datetime.datetime, move_dtstart: bool=False) -> None
    """
    Set self['DTEND'].

    If 'DURATION' in self, delete it.

    If move_dtstart, then set self['DTSTART'] such that self.duration stays the same
    """

I wonder about cases: - start - end -start, end -start, duration -start,end,duration -what is with period value type -how does it relate to repetitions?

The logic only touches DTSTART, DURATION and DTEND/DUE, so the results will be the same no matter if a RRULE is given or not.

[tobixen]

Use pythons multiple inheritance feature to add the methods just to cal.Todo and cal.Event by making them inherit

I'm not sure I feel comfortable with a MixIn-class.

Given that the code is quite straight-forward and trivial, and given that there will be some differences in the documentation (as well as the DTEND vs DUE difference), perhaps the best approach is simply to duplicate the code. I hate code duplication, but it does make the code more readable in this case.

[niccokunzmann]

If you like, you can provide a pull request with the code that you feel comfortable with, using it.

I think, I would like to use VEVENT, VTODO and VJOURNAL with the same attributes, one for start, one for end and one for duration.

What could cause misunderstanding is if the event has RDATE as a period for example. Then, it is not clear which event you mean nor that the durations would match. If this is a feature that you would like to have for the recurring-ical-events library where the results do clearly have only one duration, start and end, then you could open an issue for that one.

@tobixen, what are your thoughts on this one?

[niccokunzmann]

Otherwise, return None

I would opt for a timedelta of length zero.

Also, if the event DTSTART is on a date, the duration is one day. There are quite a few cases to test.

[tobixen]

Right, I should try to find some time for this.

I already wrote code in the caldav library, but it's not the right place for it, it's not very well designed, and it does need some more test code. I'd gladly mark them as "deprecated" and let them call on methods in the icalendar library.

What I considered is to let get_end/get_due and set_end/set_due just be aliases, allowing the end user to do event.set_due(...) or task.set_dtend(...) and the library will just do the right thing. I considered it to be an easy solution, a solution giving the best compatibility for end users (who don't need to worry if the object in the input parameter is a task or an event) and it also makes it possible to make nice code (the end user should not have to do task.set_dtend(...))