simonpoole/OpeningHoursFragment

easter-Oct Su[2]: Su,PH 11:00-17:00; "außerdem nach Vereinbarung"

Closed this issue · 12 comments

simonpoole commented

As far as I can see from http://wiki.openstreetmap.org/wiki/Key:opening_hours/specification the string seems to be invalid. Potentially Oct 1 +Su+7 days would be a valid variant, the Su[2] syntax would seem to be only valid in a weekday selector not in a date.

  • 👍1
HolgerJeromin commented

do you know what is trying to be expressed?

See the website:
"An Sonn- und Feiertagen von Ostersonntag bis zum 2. Sonntag im Oktober jeweils von 14-18 Uhr und nach Vereinbarung"

ypid commented

@HolgerJeromin You probably meant easter-Oct Su[2]: Su,PH 14:00-18:00 || "nach Vereinbarung"

Sorry for the confusion. I introduced support for this back in 2013 (ref: opening-hours/opening_hours.js@7e22871). This was based on the specification by Netzwolf, (wiki, syntax explained by Netzwolf). I later reworked the specification and created Key:opening_hours/specification based on Netzwolf’s work. Many issues where fixed by this rework, a few issues where added unfortunately. This is one of them (syntax currently not defined/missing). The spec will need to be updated. But before that it should be checked how often this is actually used. The example from @HolgerJeromin shows that there is a need for this.

HolgerJeromin commented

easter-Oct Su[2]: Su,PH 14:00-18:00 || "nach Vereinbarung" is not supported in vespucci either...

simonpoole commented

As alreday said not part of the spec at this point, and imho if included would need a close look if this is is even a sensible thing to support.

  • 👍1
simonpoole commented

@ypid while not super frequent there are likely a couple 100 of uses, could you draft a change to the specification?

ypid commented

How should we go about this? Full blown proposal process?

"couple 100 of uses" is a pretty good estimate :-)

$ ./regex_search.js export.opening_hours.json
regex search> \w{3}\s*\w{2}\[\s*-?\s*\d+\s*\]
Matched 268 different values, total in use 385.
Print values? y
Matched (count: 44): Apr Fr[-1] - Sep 30
Matched (count: 11): Apr Fr[-1] - Sep 30 00:00-24:00
Matched (count: 10): Mar Su[-1]-Oct Su[-1] -1 day: 09:00-20:30; Oct Su[-1]-Mar Su[-1] -1 day: 09:00-17:30
Matched (count: 7): Mo-Fr 10:00-20:30;Sa 09:30-18:00;Su 10:00-17:00;Jan 1 off;Feb Mo[2] off;Easter off;May Mo[-2] off;Jul 1 off;Sep Mo[1] off;Oct Mo[2] off;Dec 25 off
Matched (count: 6): Jul Su[-1] -6 days-Jul Su[-1] 08:00-20:00
[...]

Ref: https://github.com/opening-hours/opening_hours.js/blob/master/regex_search.js

simonpoole commented

@ypid given that it is simply an error that this was not included in the specification, I would think that correcting it, bumping the version and posting on tagging is appropriate.

I would just like to see a grammar beforehand, and probably some guidance on how to handle potential ambiguities, which I believe this leads to at least in simple date selector scenarios (date ranges should be distinguishable from month ranges and therefore shouldn't be an issue), for example

Apr Su[1] 10:00-12:00

This may not be an issue because while it is ambiguous in parsing, it could be that all ambiguous cases end up being functional equivalent. But that needs to be shown.

The difference in testing with my data (~150'000 unique OH strings) is a whooping 53 additional strings that can be parsed (note that I already supported the weekday[n...] element.

ypid commented

Thanks for your feedback! I will do that then later this week.