search

opening-hours / opening_hours.js

Library to parse and process the opening_hours tag from OpenStreetMap data
https://openingh.ypid.de/evaluation_tool/
222 stars 119 forks source link

It makes sense to split/refactor library into multiple according levels of compliance #143

Open d1g opened 8 years ago

d1g commented 8 years ago

70K instances of 523K are just "24/7"

I.e. if I write "if (opening_hours="24/7") { } " my code will get 13% of the data correctly.

There should be

While I appreciate efforts to support almost 100% of the data out of box, but performance hit got too big: 1k/sec vs 20k/sec in AMDmi3 version

Another suggestion is to create libraries on top of each other, so you can get 100% opening_hours library simply by chaining calls to 3 libraries rs1 -> rs2 -> rs3 without overhead of common rules in (rs1, rs2) (rs1, rs3) (rs2, rs3)

ypid commented 8 years ago

Sounds reasonable but I don’t intent to work on this for the foreseeable future as there are more important bugs/features left to do.

ypid commented 8 years ago

This is related to #150. I will keep those wishes in mind when I eventually find the time to start working on #136 and make it configurable how high the coverage should be and maybe provide support to enable/disable individual features when targeting a platform thus making it universal. If you need a small version of this library for your microcontroller project it should be possible with #136 to target C++ for example and generate a limited version.

d1g commented 8 years ago

@ypid current spec page is imprecise or incomplete

I tried to follow it as close as I could, but it led me to several unclear cases quite fast: unclear rules about additional_rule_separator and some rare syntax examples

I don't have numbers how much of valid OH library inputs it treats as valid and how much invalid OH inputs it treats as valid (false negatives due less semantic parsing).

IMO it is faster to prototype/refactor using 300L long grammar than 7.2K long implementation.

ypid commented 8 years ago

Awesome that you are interested in this subject :+1: and I find you approach of using ANTLR very interesting. I did not stumble across this project yet but I will definitely check it out. I was more looking into older parser generators like Bison and some from the Haxe world but I have not yet started to seriously work on #136. ANTLR seems to support the common programming languages which makes it suitable for the job.

current spec page is imprecise or incomplete

You are right. There are still open issues … But I guess you already know how complex this subject is and that it will probably be a life-time job at least when it should support all countries/cultures properly.

About the problem with . I would say the real problem is still the double use of the , token for this (see #86). Other then that I fine with additional rules.

Thanks for making me a contributor by the way :wink:. We can definitely move the repository over to the opening-hours organization if you like so that others can find it more easily.

d1g commented 8 years ago

@ypid, that's insane we actually have support for sunset-related events; IMO they should be separate libs (sunset lib, public hours lib) so more people can reach them, not just OSM-focused folks.

If token is ambiguous, we should change syntax to less ambiguous ('.' or '&' symbol is unused AFAIK).

PS. I just tried to move repo but GitHub said: "You don’t have admin rights to opening-hours"

ypid commented 8 years ago

If token is ambiguous, we should change syntax to less ambiguous ('.' or '&' symbol is unused AFAIK).

I already discussed this here https://github.com/opening-hours/opening_hours.js/issues/53#issuecomment-74301820. I am not against establishing a better token for this. My suggestion would be &&. You are welcome to create a proposal for this and get it standardized if the OSM community can agree on this.

that's insane we actually have support for sunset-related events; IMO they should be separate libs (sunset lib, public hours lib) so more people can reach them, not just OSM-focused folks.

I have focused on OSM and the uses for this lib I know of are all OSM related. Support for non-OSM use is not my focus but supporting it on the other hand does not hurt either.

https://github.com/opening-hours

Hm, can you join the organization, I invited you? If that does not work, you can temporarly transfer the repo to ypid as transferring between user accounts is easier.

ypid commented 8 years ago

@d1g Thanks, I moved your repository to https://github.com/opening-hours/opening_hours_grammar.

d1g commented 8 years ago

I have focused on OSM and the uses for this lib I know of are all OSM related. Support for non-OSM use is not my focus but supporting it on the other hand does not hurt either.

This feature was requested by others, it makes sense to re-use good solutions, rather than waste time re-implementing them: https://www.google.com/search?q=site:stackoverflow.com+public+holidays

I'm not fan of PHP, but http://kayaposoft.com/enrico/ is still active (from this question)

There also library that calculates "day/night/dusk/etc" states for you, but I forgot name of it.

"This repository is being transferred to @ypid" - https://github.com/d1g/opening_hours_grammar/settings

ypid commented 8 years ago

There also library that calculates "day/night/dusk/etc" states for you, but I forgot name of it.

We already got that covered: https://github.com/ypid/suncalc

This feature was requested by others, it makes sense to re-use good solutions, rather than waste time re-implementing them

I absolutely agree. I did check every module on npm a few years ago if there way some lib that could be used. There was none. I would also like to move PH and SH stuff out of the main lib to make it reusable. As there is a fair number of supported PHs by now, this could be interesting. See #71. The difficult part is to define a useful data format and decide what code/API to use.

ypid commented 7 years ago

Seems others have stepping in to help with this issue :wink: SimpleOpeningHours only covers a very small subset of the spec, which is a design goal. This should partly solve this issue. Depending on your needs, you might want to check SimpleOpeningHours. You could then use opening_hours.js as a fallback for all the crazy stuff :wink: Also refer to Other implementations in the README.

d1g commented 7 years ago

@ypid, personally I wait for implementation that would be suitable for databases

In my complete use case grammar is used (among easy tracing) to insert rules for recurring_event_specs here: http://stackoverflow.com/a/947349 Everything is calculated/precomputed inside db (PostgreSQL). Then "cached" events would be re-used in much heavy-weight tasks.

DB-wise: sunrise/sunset 1. variadic for every day 2. variadic for every object on the Planet - because of 2 they are much more demanding than other rules.

other less relevant links: 1 2 3