-
Notifications
You must be signed in to change notification settings - Fork 120
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
It makes sense to split/refactor library into multiple according levels of compliance #143
Comments
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. |
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. |
@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. |
Awesome that you are interested in this subject 👍 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.
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 <additional_rule_separator>. I would say the real problem is still the double use of the Thanks for making me a contributor by the way 😉. We can definitely move the repository over to the opening-hours organization if you like so that others can find it more easily. |
@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" |
I already discussed this here #53 (comment). I am not against establishing a better token for this. My suggestion would be
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. 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. |
@d1g Thanks, I moved your repository to https://github.com/opening-hours/opening_hours_grammar. |
This feature was requested by others, it makes sense to re-use good solutions, rather than waste time re-implementing them: 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 |
We already got that covered: https://github.com/ypid/suncalc
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. |
Seems others have stepping in to help with this issue 😉 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 😉 Also refer to Other implementations in the README. |
@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 DB-wise: |
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)
The text was updated successfully, but these errors were encountered: