-
Notifications
You must be signed in to change notification settings - Fork 479
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
Add Vanuatu holidays #1423
Add Vanuatu holidays #1423
Conversation
Pull Request Test Coverage Report for Build 5849122162
💛 - Coveralls |
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.
@Strategos1 , thank you for adding new countries to Python Holidays! Please look at suggestions below.
holidays/countries/vanuatu.py
Outdated
if year >= 1981: | ||
# Independence Day | ||
self._add_holiday("Independence Day", JUL, 30) |
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.
if year >= 1981: | |
# Independence Day | |
self._add_holiday("Independence Day", JUL, 30) | |
# Independence Day | |
self._add_holiday("Independence Day", JUL, 30) |
As we have no holidays until 1981.
holidays/countries/vanuatu.py
Outdated
self._add_christmas_day("Christmas Day") | ||
|
||
# Family day | ||
self._add_holiday("Family Day", DEC, 26) |
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.
self._add_holiday("Family Day", DEC, 26) | |
self._add_christmas_day_two("Family Day") |
holidays/countries/vanuatu.py
Outdated
# Website: https://github.com/dr-prodigy/python-holidays | ||
# License: MIT (see LICENSE file) | ||
|
||
from holidays.constants import FEB, MAR, JUL, OCT, NOV, DEC |
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.
from holidays.constants import FEB, MAR, JUL, OCT, NOV, DEC | |
from holidays.calendars.gregorian import FEB, MAR, JUL, OCT, NOV |
tests/countries/test_vanuatu.py
Outdated
def test_fater_lina_day(self): | ||
name = "Father Lini Day" | ||
self.assertHolidayName(name, (f"{year}-2-21" for year in range(1991, 2050))) | ||
self.assertNoHolidayName(name, Vanuatu(years=range(1981, 1991))) | ||
self.assertNoHoliday( | ||
f"{year}-2-21" for year in set(range(1981, 1991)).difference({1982, 1984, 2085}) | ||
) |
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.
def test_fater_lina_day(self): | |
name = "Father Lini Day" | |
self.assertHolidayName(name, (f"{year}-2-21" for year in range(1991, 2050))) | |
self.assertNoHolidayName(name, Vanuatu(years=range(1981, 1991))) | |
self.assertNoHoliday( | |
f"{year}-2-21" for year in set(range(1981, 1991)).difference({1982, 1984, 2085}) | |
) | |
def test_fater_lini_day(self): | |
name = "Father Lini Day" | |
self.assertHolidayName(name, (f"{year}-02-21" for year in range(1991, 2050))) | |
self.assertNoHolidayName(name, Vanuatu(years=range(1981, 1991))) | |
self.assertNoHoliday(f"{year}-02-21" for year in range(1981, 1991)) |
I don't see any reasons for exceptions for 1982, 1984.
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.
I copied a code from another country and forgot to change it. Sorry
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, the code is clean and tested. If no comments regarding the calendar correctness I believe it's ready for merging into beta.
Thank you @Strategos1 for such a great contribution! Great work!
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 🚀
Merged into beta, thanks for extending the list of the supported countries 👍 |
Proposed change
Your PR description goes here.
Type of change
Checklist
beta
branch of the repositorymake pre-commit
make test
,make tox
(we strongly encourage adding tests to your code)