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

US PH public holiday #69

Closed
midnightcomm opened this issue Jan 30, 2015 · 5 comments
Closed

US PH public holiday #69

midnightcomm opened this issue Jan 30, 2015 · 5 comments
Labels
prio/low type: feature Introduction of new functionality.

Comments

@midnightcomm
Copy link

Missing public holidays for United States. http://www.opm.gov/policy-data-oversight/snow-dismissal-procedures/federal-holidays/#url=2015

@ypid ypid added the type: feature Introduction of new functionality. label Jan 31, 2015
@ypid ypid added this to the v3.0.3 milestone Jan 31, 2015
@ypid ypid assigned ypid and unassigned ypid Jan 31, 2015
@ypid ypid added the prio/low label Jan 31, 2015
@ypid ypid modified the milestones: All problems solved., v3.0.3 Jan 31, 2015
@ypid
Copy link
Member

ypid commented Jan 31, 2015

Hi

Thanks for the URL but I think Wikipedia is better suited, because there are some variable holidays and I would rather like to implement the formula instead of adding them for each year 😄.

It will probably take me some time until I come to adding those holidays myself because there are other issues open that I would like to fix first. Patches are more than welcome.

@midnightcomm
Copy link
Author

Ah, thank you! Wikipedia does give a more general solution.

@maxerickson
Copy link

I put a first pass at the definitions from wikipedia here:

https://github.com/maxerickson/us_holidays

check.py is a simple lint (just loads the file as json), the output is in check.txt. I figured it made sense to work out the questions about the definitions before worrying too much about the formatting/generating a patch (using the python json library was also easier than getting node/opening_hours running). The first part of check.txt is formatted for easy comparison to the Wikipedia page. There's several special cases I didn't deal with (half days and every second year rules).

There's a lot of repetition in it right now, it mostly comes down to whether the entries should appear where they are legally defined (at the state level), or at the highest level where they always apply (seven days have the same name in all the state level jurisdictions). I also didn't try to figure out how to limit the scope of a rule at the higher level. There's some summaries of the repetition in the next part of check.txt.

The last bit of check.txt is the 'special' words used to define dates, I didn't spend a lot of time figuring out what was valid, so some of them might need adjustment.

I can do some more work to push this in the right direction, and probably a patch (but I'm hoping I can be lazy and convince someone more familiar with the project to do the patch and make sure it integrates). I'll put together some tests too.

@ypid
Copy link
Member

ypid commented Feb 12, 2015

@maxerickson Very well done. Thanks a lot 👍

There's several special cases I didn't deal with (half days and every second year rules).

I know myself that trying to model the real world with a program can get really completed. The special cases you description are new to me and can not be expressed in the json definition. *I will add them to the README, when the us holidays are included. (To be solved later).

There's a lot of repetition in it right now

You mean the redundancy in the JSON file. That is ok. I did not want to blow up the function which parses the json, so a bit of redundancy is intended in those cases.

You can define those seven days in the global definition to have some flexibility if the state is not found (new states, name changes, whatever).

I can do some more work to push this in the right direction, and probably a patch (but I'm hoping I can be lazy and convince someone more familiar with the project to do the patch and make sure it integrates). I'll put together some tests too.

That is ok with me. I will add them, when I have time …

@maxerickson
Copy link

An initial check in is now here:

https://github.com/maxerickson/opening_hours.js/tree/usholidays

There are a few things that aren't working and probably some formatting issues, but I wanted to make sure you didn't duplicate the effort. Tests are passing for most of the regions.

One issue is that the District of Columbia exists at the county level:

"address":{..."city":"Washington","county":"District of Columbia","postcode":"20500","country":"United States of America","country_code":"us"}};

I just haven't done any looking to see how to fit that into the rules hierarchy, but the tests for it crash. I pointed the top level general tests there, so they crash too, I guess they should work.

Another one is that Guam reports a bad duration that I can't make any sense of (I guess I checked in the character I had deleted to get the tests to show the intervals, but that's not a real problem).

Beyond that would be some more research to make sure the rules for each state are solid, but I think the current rules are good enough to start with.

ypid added a commit that referenced this issue Feb 15, 2015
…not defined. Related to #69.

* Also made the library more robust if state of country is somehow not included in the nominatiomJSON.
@ypid ypid closed this as completed in 2bf2aa7 Feb 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio/low type: feature Introduction of new functionality.
Projects
None yet
Development

No branches or pull requests

3 participants