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

NZ Nelson Anniversary #141

Closed
charlie-ablett opened this issue Dec 1, 2015 · 5 comments
Closed

NZ Nelson Anniversary #141

charlie-ablett opened this issue Dec 1, 2015 · 5 comments
Assignees

Comments

@charlie-ablett
Copy link

First off - thanks for your work on this awesome gem.

The NZ region file includes the Nelson Anniversary, but has the region as :nz_ak - which is Auckland (other end of the country, and has its own anniversary! - see here and here)

https://github.com/holidays/holidays/blob/master/data/nz.yaml#L42

The same goes for Taranaki, which is closer but still a separate region. According to the yml file, its anniversary also incorrectly falls under :nz_ak.

It's not clear which is the Nelson region - :nz_nl seems to be for Northland. I'd be happy to make a change to the data files themselves, but not confident I'd test them properly. Based on how I think things work, it might look something like:

2:
  - name: Nelson Anniversary Day
  regions: [nz_ne]
  mday: 1
  observed: closest_monday

and

3:
  - name: Taranaki Anniversary Day
  regions: [nz_ta]
  week: 2
  wday: 1
  observed: closest_monday

Thanks muchly!

@ppeble
Copy link
Member

ppeble commented Dec 5, 2015

Thank you for this! I have a few NZ coworkers and if they knew that this gem had incorrect holidays for their homeland I might get smacked. 😄

Let me take a look at this and see if I can understand the issue. I'll dig into those 'public holidays' links, they usually make things clear enough to me.

@charlie-ablett
Copy link
Author

Thanks! :D

@ppeble
Copy link
Member

ppeble commented Dec 11, 2015

While attempting to implement this I think I've uncovered a bug in the date processing for observed holidays that fall at month end or month beginning. Based on my understanding the current logic won't allow for the 2017 Nelson's holiday. If you give 2017/1/30 (the correct date according to this) then it doesn't find it. I think I see the issue but I'll need to think about what I can do here.

I'm going to push up the definition updates that you specified but then open another issue to deal with the bug that I think I see. I'll roll that change in with the refactor branch I have, which I plan on merging soonish. The good news is that 2016 seems fine, it's just 2017 that I noticed the issue.

PR coming soon!

@ppeble
Copy link
Member

ppeble commented Dec 11, 2015

Here is the PR: #145

Can you please take a quick look and let me know if I messed anything up?

@ppeble
Copy link
Member

ppeble commented Dec 13, 2015

I went ahead and merged. If there are issues we can address them in a new PR. I am going to pull down these changes and try to fix the other issue that I opened in my refactor branch. Thanks again for taking the time to submit the issue!

@ppeble ppeble closed this as completed Dec 13, 2015
Murphydbuffalo pushed a commit to Murphydbuffalo/holidays that referenced this issue Sep 21, 2021
Add Festa di San Giovanni Battista
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants