feat: Allow skipping bad events instead of raising InvalidCalendar

[fabien-michel]

Add a skip_bad_events option allowing to silently skip event if an InvalidCalendar error is raise during process.

This partially addresses #132 by skipping the event with bad date order.

May-be we should wait the work on #133 PR which may handle better options management.

[niccokunzmann]

Short: If you find that this is a use case and so, I understand it, I would rather merge it than discuss too much. Please continue contributing and this library will get better.

---

Long:

A note on how the assertions come to be here that we replaced with the errors in #134. This is a sentence in my head remembering the decision, I think, the knowledge originates from the an OOPSLA conference I attended.

Assertions and tests complement each other: While tests make sure that the code works as expected, assertions in the code make sure that the usage of the code does not step out of the assumptions made.

In that sense: I made the assumption that DTSTART is before DTEND when I programmed the library because I do not want to program in assumptions for which I do not have data. As such, the assert that was converted to an exception in #134 is slightly misrepresenting what the assertion was meant to be:

• Assertion: We do not know about this, so when it happens, please report back and we will find a way to deal with this.
• Error: We have considered this use case and we decided that this is the way to handle it.

As such, I do not agree to the semantic change that the exception has created.

I would say:

1. Nobody really encounters these errors, you are the first to bother report them. They did not occur in the Open-Web-Calendar logs when I was scanning for errors.
2. This library's intention is to make it easy for people to use ICS calendars. It adds X-WR-TIMEZONE support so that people do not have to bother with Google making an exception to the ICS standard. So, x_wr_timezone is a bolder move than this PR: There is no documented parameter for people to opt out from handling Google calendars in a special way. In a similar sense, I would want the most ease in default behaviour: skip=True
3. Looking at humans, please tell me when I am mistaken, this is my impression: You are a skilled programmer. You made a good first PR, tested your code, you went far without help. Additionally, to me, it looks as if you would really like to be considerate, not upset people and not break code or people's assumptions when using it. Contrary to this, I read a bit much of Peter Hintjens: http://hintjens.com/blog:_start With that in mind, I know, I like to be perfect but if I ship problems, people (you are one) come back and solve them. Bugs also make FOSS work!
4. Elaborating on 2&3: You are the person to decide how to approach this one calendar creator who swaps start and end and that one, from my point of view, did not do the work as properly as you would have done had you created the software (do TDD, swapped where to subtract the duration, something like this). What is of value to you is of value to everyone. I see you added a parameter with a value to not break the assumed default behaviour (I assume). Then, the value is for you in the skipping. You having a value is value enough to ship this. Your value could be the parameterless default and it would be the right choice for at least one person.

If I want to improve it, I can do that. Anyone can. Best if it is shipped and people can add on.

---

Now the viewpoints that should not lead to a prolonging of a merge:

1. Personally, I might create a PR that swaps start and end because I think that something ending before the start makes no sense and someone probably has just swapped values, especially seeing that they are 30 min apart and it is not meant that the event occurs from the start of times to the DTEND end then from DTSTART to the end of times. So, from my point of view skipping these events is worse than adding them slightly modified and waiting for feedback after 5 years. That is though a value judgement and since I do not do the work yet, I will still merge it and we improve step by step. Value added bit by bit.
2. I use this library without any config parameters here:
   https://github.com/niccokunzmann/open-web-calendar/blob/fc14cea376e506d9c049ef8127ebc37d1c40884b/convert_to_dhtmlx.py#L128
   My other interest is: I would like the open web calendar to support weird calendars by default so that they render as expected.

---

Thanks!

[niccokunzmann]

Also, please feel free to follow https://github.com/niccokunzmann/python-recurring-ical-events?tab=readme-ov-file#new-releases
I will do it randomly and you could have a release tomorrow if you start it! 🎉

[fabien-michel]

Thanks taking time to respond !

I understand you don't want to pass more time on this but I need to clarify things.

I feel confident enough to suggest modifications, but I'm clearly not skilled to take decisions on what is good for this library. I'm glad you open write access to me but I don't feel comfortable/legitimate making changes without your approval.
I'm using for only two weeks, and I'm not familiar with all ical things at all.

I thinks it's up to you (the maintainers) to drive the pull requests so they fits to your long-time view.

For the technical point:

For my particular use case, It's occurred twice on the "only" 30 calendars (all on Sogo) I'm using this lib on, and it make it to crash because of an assert so my whole process isn't working.
I will soon try this lib with a wide variety of calendar providers. I need to have a way to bypass bad formatted events so the lib can handle all events it is possible to do without crashing.

I've exposed my arguments on the #132 issue (the lib should not output modified data as it is true data). But I understand I'm the only one who experience this and it is not a big deal so I'm totally ok for swapping start/end.

Concerning assert, IMHO they should only be for a misuse of the lib API by a developer.
Here I have wrong data as input so an Exception make more sens. (same for potential bad RRULE format)
Futhermore, assert are stripped out when python run with optimized bytecode -O, so the code might still crash with an Exception.

Thanks !

[niccokunzmann]

Okay! Cool! Thanks for your reply!