Does this actually fix #401, or just stop #401 from raising an error? I agree that the RFC says that this VALUE=DATE-TIME is a valid value for the parameter, but it seems weird to just ignore it. Is it somehow superfluous information?

Is it somehow superfluous information?

Yes. The type of DTSTART can be either DATE-TIME or DATE. DATE-TIME is the default and supported by dateutil. DATE is not supported and not allowed for DTSTART inside of VTIMEZONE.

OK, when I have a bit of time I'll check the RFC to double-check, but if so this is fine.

OK, when I have a bit of time I'll check the RFC to double-check

rfc5545-3.8.2.4:

3.8.2.4.  Date-Time Start

   Property Name:  DTSTART
...
   Value Type:  The default value type is DATE-TIME.
...
   Format Definition:  This property is defined by the following
   ...
                  (";" "VALUE" "=" ("DATE-TIME" / "DATE")) /
                  (";" tzidparam) /

rfc5545-3.6.5:

3.6.5.  Time Zone Component

   Component Name:  VTIMEZONE
...
   Note:
   ...
      The mandatory "DTSTART" property gives the effective onset date
      and local time for the time zone sub-component definition.
      "DTSTART" in this usage MUST be specified as a date with a local
      time value.

What's interesting here is that in this specific if/elif block, other name values explicitly support VALUE=DATE-TIME, so whoever originally implemented this (I assume Gustavo) knew about these parameters and explicitly did not implement it for DTSTART.

Ah, I see the issue. rrule follows the RFC 2445 spec, which was later superceded by RFC 5545. RFC 2445 does not support VALUE=DATE-TIME, but RFC 5545 does.

OK, I've got my head around how this works now. Thanks for the PR and report @Unrud.

I don't think that VALUE=DATE is allowed.

"DTSTART" in this usage MUST be specified as a date with a local time value.

@Unrud I was just going by the bit you quoted above:

> ";" "VALUE" "=" ("DATE-TIME" / "DATE")) 

Seems to say it can be a date or a datetime, and can have a TZ-ID parameter as well. I'm not sure exactly what to make of the otherparam part. I'm fairly confident that DATE is allowed, though, since in RFC7529 (an update to RFC 5545), many examples use VALUE=DATE.

@Unrud I was just going by the bit you quoted above:

> ";" "VALUE" "=" ("DATE-TIME" / "DATE"))

The quote is about DTSTART in general. But the same comment also contains a second quote, that is specific to DTSTART inside of VTIMEZONE.

and can have a TZ-ID parameter as well.

Not if it's inside of VTIMZEONE:

"DTSTART" in this usage MUST be specified as a date with a local time value.

Hm.. I think that needs to be fixed in tz.tzical, since rrulestr definitely supports it.

Actually, just tested it, rrulestr is not affected by this PR, because tzical fails if DTSTART provides any parameters:

from dateutil import tz
from io import StringIO

TZICAL_EST5EDT = """
BEGIN:VTIMEZONE
TZID:US-Eastern
LAST-MODIFIED:19870101T000000Z
TZURL:http://zones.stds_r_us.net/tz/US-Eastern
BEGIN:STANDARD
DTSTART;VALUE=DATE-TIME:19671029T020000
RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=10
TZOFFSETFROM:-0400
TZOFFSETTO:-0500
TZNAME:EST
END:STANDARD
BEGIN:DAYLIGHT
DTSTART:19870405T020000
RRULE:FREQ=YEARLY;BYDAY=1SU;BYMONTH=4
TZOFFSETFROM:-0500
TZOFFSETTO:-0400
TZNAME:EDT
END:DAYLIGHT
END:VTIMEZONE
"""

tz.tzical(StringIO(TZICAL_EST5EDT))   # Raises ValueError.

Nope, my bad, was using the release branch, on master this is indeed using the standard call.

Hm.. I think that needs to be fixed in tz.tzical, since rrulestr definitely supports it.

My bad! I thought that these functions are exclusively used to parse VTIMEZONE components, because they seem to lack support for the DATE type or TZID that would be required otherwise.