-
Notifications
You must be signed in to change notification settings - Fork 462
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
Update Thailand holidays: add holiday categories #1346
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! 🚀 I'm happy that my idea with categories was useful. Now I have extended it to special holidays as well.
Co-Authored-By: ~Jhellico <KJhellico@users.noreply.github.com>
Co-Authored-By: ~Jhellico <KJhellico@users.noreply.github.com>
Co-Authored-By: ~Jhellico <KJhellico@users.noreply.github.com>
Not sure if it's still WIP. Please update the description if it's ready for review. |
@arkid15r I'm afraid this is still very WIP as I've yet got the time to implement their National Date of Importance List for workday category in full |
tests/common.py:237: in _assertHolidays
self.assertEqual(
E AssertionError: 2287 != 28 : {('2021-02-12', 'วันหยุดพิเศษ (เพิ่มเติม)'), ('2045-01-14', 'วันเด็กแห่งชาติ'), ('1975-05-05', 'วันฉัตรมงคล'), ('1990-04-13', 'วันสงกรานต์'), ('2000-04-13', 'วันสงกรานต์'), ... Maybe I should just restart this as a new PR at this rate |
Hey @PPsyrius, it's totally fine to close it in favor of a new one or continue your work within this PR. Whatever works best for you. Thank you 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And test is pass.
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com>
Pull Request Test Coverage Report for Build 5748522614Details
💛 - Coveralls |
Add categories support to subdivision holidays methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Are you planning to add any more holidays?
Co-Authored-By: ~Jhellico <KJhellico@users.noreply.github.com>
On additional |
I think, it's time to merge this PR. |
Well, it's up to you. I don't see any reasons to not make it ready for review if you feel so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🇹🇭 LGTM
This is another great update, I appreciate your work @PPsyrius. This PR is in the merge queue and will be submitted automatically once tests are passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needed another approval though
Proposed change
Following the changes in #1320, Thailand holidays now support holiday categories.
Change List from 0.27 Release (More incoming, still WIP)
ARMED_FORCES:
+ Royal Thai Armed Forces Day
BANK:
+ Additional Closing Day for Bank for Agriculture and Agricultural Cooperatives
+ Mid-Year Closing Day
GOVERNMENT:
+ Royal Ploughing Ceremony
(Moved fromPUBLIC
since this only apply to government employees)PUBLIC (Old Default Category):
+ National Children's Day
(Turns out these are National Public Holidays rather than just Observed National Days)- Royal Ploughing Ceremony
(see above)SCHOOL
+ Teacher's Day
WORKDAY
+ Thai Veterans Day
+ National Science Day
+ National Artist Day
+ International Women's Day
+ National Forest Conservation Day
+ HM King Ramkamhaeng Memorial Day
+ National Aviation Day
+ Thai National Flag Day
+ Loy Krathong
Type of change
Checklist
beta
branch of the repositorymake pre-commit
make test
,make tox
(we strongly encourage adding tests to your code)