Add Hong Kong calendar. Factor out the Christmas/Boxing Day Sunday s… #235
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
June 25, 2017 18:59
…ift as its used in the UK and in HK. Add a test to Poland as an example to ensure it *doesn't* shift countries which don't shift Christmas. Add complicated Sunday shift for the third day of CNY - I would rather loop this than hardcode it, but I didn't want to change the core approach until it had been discussed.
…r before the PR (hence why I split it)? Probably something obvious I missed.
|
It's weird to have all these changes at once, but it looks fine: thanks to our tests, it's obvious that you didn't break a thing! Don't worry about the fact that it's "hardcoded" in some way, we can refactor this bit later on. |
|
Yeah sorry I'll try to break things up better going forward.
…On Mon, 26 Jun 2017 at 16:17, Bruno Bord ***@***.***> wrote:
It's weird to have all these changes at once, but it looks fine: thanks to
our tests, it's obvious that you didn't break a thing!
Don't worry about the fact that it's "hardcoded" in some way, we can
refactor this bit later on.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#235 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ATTDfOXDVWVgce0G47CNBx28f_jgHO7Rks5sH2k3gaJpZM4OEk6o>
.
|
brunobord
requested changes
Jun 26, 2017
| @@ -327,6 +327,19 @@ def get_whit_sunday(self, year): | |||
| def get_corpus_christi(self, year): | |||
| return self.get_easter_sunday(year) + timedelta(days=60) | |||
|
|
|||
| def shift_christmas_boxing_days(self, year): | |||
There was a problem hiding this comment.
I think you can add a docstring here, for more clarity. it's not a blocking point, though
workalendar/core.py
Outdated
| if christmas.weekday() in self.get_weekend_days(): | ||
| shift = self.find_following_working_day(christmas) | ||
| results.append((shift, "Christmas Shift")) | ||
| results.append((shift + timedelta(days=1), "Boxing Day Shift")) |
There was a problem hiding this comment.
I'd like to see the following label here: "{} Shift".format(self.boxing_day_label)
In some countries, Boxing Day is named "St Stephen's" or "The Day After Christmas", for example
workalendar/core.py
Outdated
| results.append((shift + timedelta(days=1), "Boxing Day Shift")) | ||
| elif boxing_day.weekday() in self.get_weekend_days(): | ||
| shift = self.find_following_working_day(boxing_day) | ||
| results.append((shift, "Boxing Day Shift")) |
added 3 commits
July 2, 2017 21:06
…1, if CNY started on a Sunday it was rolled to a Saturday. This was changed in 2011 since most people had Sat off anyone, and so they made it the following Wed. How very kind of them. Also captured the edge case when chingming falls on the same day as another holiday (most likely a Christian variable holiday such as Easter).
…o hong-kong * 'master' of https://github.com/novafloss/workalendar: Splitted `africa.py` file into an `africa/` module
|
Hey - sorry for the delay, real life got in the way ;-) OK I've done the requested changes, plus also fixed a couple of other edge cases I found while researching the Hong Kong calendar (see commit message for details). |
brunobord
approved these changes
Jul 3, 2017
|
Thanks a lot for this contribution! |
|
My pleasure! I look forward to being able to contribute more. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…hift as its used in the UK and in HK. Add a test to Poland as an example to ensure it doesn't shift countries which don't shift Christmas. Add complicated Sunday shift for the third day of CNY - I would rather loop this than hardcode it, but I didn't want to change the core approach until it had been discussed.
refs #
For information, read and make sure you're okay with the CONTRIBUTING document.
<country>by@pseudo(#)"