Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resurveying opening hours #1651

Closed
matkoniecz opened this issue Nov 15, 2019 · 9 comments
Closed

resurveying opening hours #1651

matkoniecz opened this issue Nov 15, 2019 · 9 comments
Labels
new quest accepted new quest proposal (if marked as blocked, it may require upstream work first)

Comments

@matkoniecz
Copy link
Member

based on more general #1112 #1364

In context of resurveying things opening_hours tag is a bit special:

  • it goes out of date relatively quickly
  • entering this data takes long time, so "reask question every X years" is not feasible unlike most of other quests
  • has quite complex syntax

To handle it properly and to not end with something horrible for start it would be nice to have something along lines: "Is following opening hours still correct for $NAME?" and show opening hours data.

On answering yes, it would add check_date:opening_hours.

On answering no it would delete opening_hours tag and check_date:opening_hours if present, what should trigger opening hours quest allowing to enter correct data.


Most of the fun is in "show opening hours data". Raw tag value is used in test version, but it is obviously not a proper solution, parsing opening hours data into some readable format is necessary, with unparsable opening hours fields ignored.

For parsing I see two solutions - (1) follow JOSM and embed js code from https://github.com/opening-hours/opening_hours.js and use it for parsing or (2) write a new parser from scratch, starting from one handling a small subset of opening_hours syntax.

@westnordost For me both versions sounds a bit frustrating and I have no strong preference - do you have any preference toward either one? I predict that running js code within SC may have some issues with getting more dependencies (js library itself, runner of js code that would be probably Rhino or an other Java library) while new parser will take more time to recreate something already written.


Working on this was triggered by going to a shop and discovering there that it is closed because OSM data for opening_hours was old and wrong.

@westnordost westnordost added the new quest accepted new quest proposal (if marked as blocked, it may require upstream work first) label Nov 16, 2019
@westnordost
Copy link
Member

On answering no it would delete opening_hours tag and check_date:opening_hours if present, what should trigger opening hours quest allowing to enter correct data.

Showing the input for new opening hours right away would also be an option. That's certainly what the user would expect.

My preference is to have a parser written "from scratch" that only understands what the app can also display. Because, even if you took that javascript-parser-version, or any other, you'd still need a validator that checks whether the syntax can be displayed in StreetComplete.
On the other hand, a piece of code that just turns the OSM-opening hours syntax into something human-readable + localized is fine as well, but we miss the opportunity to present the user directly with a WYSIWYG form to update the opening hours.

@matkoniecz
Copy link
Member Author

Because, even if you took that javascript-parser-version, or any other, you'd still need a validator that checks whether the syntax can be displayed in StreetComplete.

It sounds relatively simple to check whatever data in some reasonable structured format meets SC requirements.

I looked a bit more and found https://github.com/simonpoole/OpeningHoursParser that seems promising.

My preference is to have a parser written "from scratch"

Do you have any preference/advise for which kind of parsing library would be a good idea? I had some limited experience with parsing and not using parsing library from start was a big mistake.

opportunity to present the user directly with a WYSIWYG form to update the opening hours.

That would be the ideal form.

@westnordost
Copy link
Member

https://github.com/simonpoole/OpeningHoursParser indeed looks very good! The parsing result can then be scanned for elements that can not be represented in the form in StreetComplete quite easily!

If this library is used, then the current data structures for the model (not the view model) should probably be replaced.

@matkoniecz
Copy link
Member Author

There are some values that are gibberish or using syntax not supported by StreetComplete. Such opening hours will not be supported and due to both complexity and rarity are probably not worth supporting.

But there are also some opening hours that can be entered with StreetComplete but also seem not worth supporting. I listed some spotted so far, I would welcome idea if someone has a good way to support some of them or feels that supporting them is necessary.

This issues are related to intentional limitations of opening hours edit form. It is able to display certain subset of opening hours, in some cases it may be a bit surprising.

  • 24/7 can't be displayed. Closest displayable form is Mo-Su 00:00-24:00.
    • solution 1: do not support opening_hours=24/7 in resurvey opening hours quest (planned)
    • solution 2: add 24/7 value display support in opening hours edit box
    • solution 3: add 24/7 value support just in the resurvey quest – but what should be done with users that will want to mark „except public holidays” in reaction to „Is it open all day, every day?” (Allow setting off on opening hours #276)
    • solution 4: silently convert to Mo-Su 00:00-24:00 - seems simply a a bad idea
  • PH off part is not displayable (Allow setting off on opening hours #276). So on resurveying opening hours with this part it would not be possible to display and verify this part.
    • solution 1: do not support such opening hours and skip them (planned)
    • solution 2: assume that PH off is always correct and do not display it to user, then silently add it back (will cause problems sooner or later)
    • solution 3: display "closed on public holidays" without way to add it - it would be confusing for users
  • mixing month indexed and full year rules (Th 17:30-19:30; Jul-Sep Th 17:00-19:00) StreetComplete supports month-based opening hours but in such cases all opening hours need to be specified for months.
    • solution 1: do not support such opening hours and skip them (planned)
    • solution 2: Automatic converting (may cause problems and is not worth writing as such rules seem to be extremely rare)
  • implied 24/7 (Jan-Dec rule types)
    • solution 1: skip such rules (planned)
  • opening_hours tags with overriding rules (Th 17:30-19:30; Th 17:00-19:00)
    • solution 1: detect and skip such rules (planned)
    • solution 2: drop overiden parts and ask whatever such opening hours are correct, save confirmed version if accepted (error prone to support values that are bogus and suspect)

@westnordost
Copy link
Member

You can always start with supporting less and adding support for more things later. This is what I recommend anyway, since the initial effort is big enough anyway, as you need to probably replace all the data structures currently used for the model with the ones from that library.

In detail regarding you questions:

  • Nothing should be silently converted
  • 24/7 could be supported by just showing the same text in big letters instead of the opening hours input that you see in the other answers menu. If the answer is "no, they are not correct anymore", show the normal OH form
  • regarding off-times, this would be something where the OH form itself could be extended later some time
  • regarding overriding rules (;), this presents a bit of a problem, because most rules are indeed written like this. IIRC StreetComplete also uses the ; operator if the selected weekday ranges do not overlap

Anyway, two things I want to comment on in general:

  1. You probably see now that the complexity to do this is quite high, so don't you think that there are other things with a higher ROI that could be done first?
  2. You have a whole lot of open construction sites where you said you would want to do it and partly I see already some unfinished implementation in your repos or you even posted a PR for this. Don't you want to finish what you started first? After all, it's your choice what things you give priority to but if you "reserve" a task for yourself, it means that I refrain from implementing it. If you never finish, but instead start new projects, it kind of blocks the progress

@matkoniecz
Copy link
Member Author

Nothing should be silently converted

What about lossles conversions, without changing structure?

For example:

  • Th 17:30-19:30 open changed to Th 17:30-19:30
  • Mo,Tu,We 9:00-10:00 -> Mo-We 9:00-10:00
  • Mar-Oct Tu-Su 10:30-18:00; Mar-Oct Mo 10:30-14:00; Nov-Dec Tu-Su 11:00-17:00; Nov-Dec Mo 11:00-14:00 changed to Mar-Oct: Tu-Su 10:30-18:00;Mar-Oct: Mo 10:30-14:00;Nov-Dec: Tu-Su 11:00-17:00;Nov-Dec: Mo 11:00-14:00

Here I think that converting should be OK. If not, it would be fairly easy to add one more filter excluding tags where "nothing is changed" answer would edit opening_hours tag. But I think that it is OK to make such purely cosmetic changes without significant rearranging of structure, so current code is not excluding this.


as you need to probably replace all the data structures currently used for the model with the ones from that library

To make things simpler I ended with:

  • parsing using library
  • creating object using library structure but limited to only supported functionality and checking whatever it is the same (this catches all valid but unsupported syntax like Feb+ Mo[1] 09:00-18:30)
  • creating OH object for use by StreetComplete editing panel from Vespucci oh structure

Objects returned by this parsing library are designed to represent all kinds of opening hours including things like 2012-2022 Feb+ Mo[1] -5 days 09:00-(sunset+00:50). And current SC version fits well model of subset actually displayable in SC.

24/7 could be supported by just showing the same text in big letters instead of the opening hours input that you see in the other answers menu. If the answer is "no, they are not correct anymore", show the normal OH form

I worry here that reaction may be „it is open all the time, but not during PH” and then discover that it is not specifiable (I even attempted initially to use „PH 06:00-06:00” to specify „PH off”). But anyway, for now I decided to skip this in the initial version. I prefer to limit myself to reusing an existing UI.

regarding overriding rules (;), this presents a bit of a problem, because most rules are indeed written like this. IIRC StreetComplete also uses the ; operator if the selected weekday ranges do not overlap

For rules that target ranges without overlap is perfectly OK and valid. Fun appears where ranges overlap, but I have now code that successfully detects such opening hours and refuses to process them.

You probably see now that the complexity to do this is quite high, so don't you think that there are other things with a higher ROI that could be done first?

That is why first made simple prototype first – with display of raw opening hours tag (Is <opening_hours> still correct for <name>?). It turned out to catch many, many outdated opening hours entries and POIs that are now gone.

I think that it is a really good idea. Also, I may be still bitter that I arrived to a closed IKEA because I trusted opening hours from OSM.

You have a whole lot of open construction sites where you said you would want to do it and partly I see already some unfinished implementation in your repos or you even posted a PR for this. Don't you want to finish what you started first? After all, it's your choice what things you give priority to but if you "reserve" a task for yourself, it means that I refrain from implementing it. If you never finish, but instead start new projects, it kind of blocks the progress

I noticed the problem. And I went with this one as I though about finishing one of open tasks where I spend some time on digging around. Though now I see that it would be better to finish one of cases where I have nearly finished code except some small irritating part.

@NGC935
Copy link

NGC935 commented Jun 16, 2020

I stopped writing an issue because of this one, so my question is: what's the state of tje opening hours? Background is here, that in 4 of 5 cases when i try to answer this quest, it's more complex than 8h/day from Mo-fr. So i downloaded Vespucci to read other opening information and added stuff to my own changes, not very comfortable. Is it or will it be possible to select many different hours for this quest?
If.not, I'll write my own issue, np.

@westnordost
Copy link
Member

I believe this is the wrong issue. Anyway, it is possible with Streetcomplete to add hours that are more complex than 8h/day from Mo-Fr, example:

@matkoniecz
Copy link
Member Author

implemented!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new quest accepted new quest proposal (if marked as blocked, it may require upstream work first)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants