search

collective / icalendar

icalendar parser library for Python
https://icalendar.readthedocs.io
Other
1k stars 172 forks source link

VTODO - start, end, duration #733

Closed niccokunzmann closed 1 month ago

niccokunzmann commented 1 month ago

This is a contribution to #496.

@tobixen, could you help me understand this a bit better: What I do not know about VTODO is commented here, too. I am unclear about duration computation, too if DUE only or START only is set, it seems there is no duration, then. In this way, this is very different from an event. Could we talk about edge cases and such? I do not use VTODO but would like to have that implemented and known so that I can work on #716 and https://github.com/niccokunzmann/python-recurring-ical-events/issues/186.

Your feedback would be much appreciated.

002aa22 treats VTODO like VEVENT but it seems to me, there are slight differences.

tobixen commented 1 month ago

I am unclear about duration computation, too if DUE only or START only is set, it seems there is no duration, then. In this way, this is very different from an event. Could we talk about edge cases and such? I do not use VTODO but would like to have that implemented and known so that I can work on #716 and niccokunzmann/python-recurring-ical-events#186.

  • [ ] no properties

If no properties are set, then all three (DTSTART, DUE, DURATION) are undefined. Setting one of the attributes should cause this attribute to be set in the returned object.

  • [ ] DTSTART only

DURATION is undefined. If DURATION is set, that should be considered equivalent to setting a DUE-date. It's probably appropriate to return an object with DTSTART and DURATION set in that case.

  • [ ] DUE only

DURATION is undefined. If DURATION is set, the object has to be populated with a DTSTART. Should the object be returned with a DUE or a DURATION? Those are mutually exclusive, so only one of the attributes should be returned. I'm in favor of keeping DUE.

  • [ ] COMPLETED only

DURATION is undefined. If DURATION is set, then populate the object with DURATION ... and I'd say, consider the DURATION to be time tracking data, and populate it with a DTSTART as well.

  • [ ] DUE vs. COMPLETED for duration or both
  • [ ] DURATION vs. DUE in case of COMPLETED

I've considered that DURATION should be used for the time estimate and not time tracking, from that perspective, ignore the COMPLETED for an object with both DUE and COMPLETED set. And also, since DUE and DURATION is mutually exclusive ... IMO that gives a strong hint that the intention is that one should consider DUE to be equivalent with DTSTART+DURATION.

  • [ ] any other attributes that mark when a TODO starts or ends

There aren't - at least not as I'm aware of. (perhaps some new RFCs have been posted since last I was studying this though).

tobixen commented 1 month ago

If DURATION is defined in the object and one sets a new DUE, then I'd say the DTSTART should be moved rather than changing the DURATION.

If DTSTART and DUE is defined, and one attempts setting DURATION, then I also think DTSTART also should be moved.

This fits in with my use-cases ... though perhaps it's just my use-cases, I don't have strong opinions that it has to be this way. As said, I've chosen to define the DURATION as the time estimate, to consider DUE to be either a hard due or wishful thinking on when the task should be completed. Assuming those writing the standard considered DUE and DTSTART+DURATION to be equivalent, DTSTART is then efficiently the last possible moment one can start working on the task and theoretically complete it in time - assuming that the person working on the task does not need to sleep etc.

With my definitions, DUE may be important, DURATION may be a well thought-through variable, while the DTSTART is a value that does not have a strong real-world meaning (One ought to start with the task earlier than the defined DTSTART the way I consider those properties to be defined).

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 11383520223

Details

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/icalendar/cal.py 65 67 97.01%
<!-- Total: 207 209 99.04% -->
Totals Coverage Status
Change from base Build 11369442799: 0.04%
Covered Lines: 3602
Relevant Lines: 3690
💛 - Coveralls
niccokunzmann commented 1 month ago

@tobixen Could you give this a review?

Here, I basically use the same algorithms in Event start & end for Todo start & end.

Altough they are different, I wonder how to deal with the differences.

There are two things to notice:

I went through your requirements again.

Edge case to consider: For VTODO, DURATION seems to be ok to be set by itself without DTSTART. Is that true? In that case, we are not correct here for the Todo.duration attribute.

niccokunzmann commented 1 month ago

@stevepiercy, thanks for the review. I applied the changes :)

niccokunzmann commented 1 month ago

Readthedocs: https://icalendar--733.org.readthedocs.build/en/733/

niccokunzmann commented 1 month ago

@stevepiercy The documentation looks better and better: :heart: https://icalendar--733.org.readthedocs.build/en/733/api.html#icalendar.cal.Event

niccokunzmann commented 1 month ago

In the end, and if it makes sense, I would like to have start, end and duration properties on all components. Missing to evaluate:

niccokunzmann commented 1 month ago

@tobixen I will merge it for now. Please use it one day and create a PR in case you notice how to improve it. @stevepiercy Thanks for your review. I added your suggestions.

Over a week has passed and I would like this functionality for #737.