Added Turkmenistan holidays #2416
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis pull request introduces support for Turkmenistan holidays into the library. An import statement has been added for Turkmenistan in the countries module. A new file defines the holiday logic for Turkmenistan, detailing public holidays starting from 1992 and including alias classes. Additionally, a new test suite has been created to validate holiday calculations for various years. Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)holidays/countries/__init__.py (1)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/__init__.py(1 hunks)holidays/countries/turkmenistan.py(1 hunks)tests/countries/test_turkmenistan.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
holidays/countries/__init__.py (1)
holidays/countries/turkmenistan.py (3)
Turkmenistan(6-63)TM(65-66)TKM(68-69)
tests/countries/test_turkmenistan.py (1)
holidays/countries/turkmenistan.py (1)
Turkmenistan(6-63)
🪛 Ruff (0.8.2)
holidays/countries/turkmenistan.py
1-1: holidays.calendars.gregorian.JAN imported but unused
Remove unused import
(F401)
1-1: holidays.calendars.gregorian.MAR imported but unused
Remove unused import
(F401)
1-1: holidays.calendars.gregorian.MAY imported but unused
Remove unused import
(F401)
1-1: holidays.calendars.gregorian.SEP imported but unused
Remove unused import
(F401)
1-1: holidays.calendars.gregorian.OCT imported but unused
Remove unused import
(F401)
1-1: holidays.calendars.gregorian.DEC imported but unused
Remove unused import
(F401)
15-15: Trailing whitespace
Remove trailing whitespace
(W291)
19-19: Trailing whitespace
Remove trailing whitespace
(W291)
26-26: Blank line contains whitespace
Remove whitespace from blank line
(W293)
28-28: Blank line contains whitespace
Remove whitespace from blank line
(W293)
32-32: Blank line contains whitespace
Remove whitespace from blank line
(W293)
35-35: Blank line contains whitespace
Remove whitespace from blank line
(W293)
36-36: Trailing whitespace
Remove trailing whitespace
(W291)
38-38: Trailing whitespace
Remove trailing whitespace
(W291)
40-40: Blank line contains whitespace
Remove whitespace from blank line
(W293)
46-46: Blank line contains whitespace
Remove whitespace from blank line
(W293)
47-47: Trailing whitespace
Remove trailing whitespace
(W291)
49-49: Blank line contains whitespace
Remove whitespace from blank line
(W293)
60-60: Blank line contains whitespace
Remove whitespace from blank line
(W293)
69-69: No newline at end of file
Add trailing newline
(W292)
tests/countries/test_turkmenistan.py
9-9: Trailing whitespace
Remove trailing whitespace
(W291)
10-10: Trailing whitespace
Remove trailing whitespace
(W291)
30-30: Blank line contains whitespace
Remove whitespace from blank line
(W293)
37-37: Blank line contains whitespace
Remove whitespace from blank line
(W293)
46-46: Blank line contains whitespace
Remove whitespace from blank line
(W293)
51-51: Blank line contains whitespace
Remove whitespace from blank line
(W293)
56-56: Blank line contains whitespace
Remove whitespace from blank line
(W293)
58-58: Line too long (105 > 99)
(E501)
59-59: Blank line contains whitespace
Remove whitespace from blank line
(W293)
62-62: Blank line contains whitespace
Remove whitespace from blank line
(W293)
65-65: Blank line contains whitespace
Remove whitespace from blank line
(W293)
72-72: Trailing whitespace
Remove trailing whitespace
(W291)
73-73: Trailing whitespace
Remove trailing whitespace
(W291)
74-74: Blank line contains whitespace
Remove whitespace from blank line
(W293)
77-77: Blank line contains whitespace
Remove whitespace from blank line
(W293)
80-80: Blank line contains whitespace
Remove whitespace from blank line
(W293)
81-81: Line too long (114 > 99)
(E501)
86-86: No newline at end of file
Add trailing newline
(W292)
🔇 Additional comments (7)
holidays/countries/__init__.py (1)
163-163: LGTM! Turkmenistan import added properly.The import for Turkmenistan and its aliases (TM, TKM) has been correctly placed in alphabetical order, following the established pattern in this file.
holidays/countries/turkmenistan.py (3)
6-16: Good implementation of Turkmenistan holidays class.The class follows the project's patterns with proper documentation, country code, and starting from 1991 when Turkmenistan gained independence.
🧰 Tools
🪛 Ruff (0.8.2)
15-15: Trailing whitespace
Remove trailing whitespace
(W291)
17-21: Correct initialization of parent classes.The constructor properly initializes all parent classes in the right order.
🧰 Tools
🪛 Ruff (0.8.2)
19-19: Trailing whitespace
Remove trailing whitespace
(W291)
57-63: Appropriate implementation of Islamic holidays.Adding three days for each Eid holiday aligns with Turkmenistan's observance practices.
🧰 Tools
🪛 Ruff (0.8.2)
60-60: Blank line contains whitespace
Remove whitespace from blank line
(W293)
tests/countries/test_turkmenistan.py (3)
1-11: Good test setup with representative years.The setup initializes holiday objects for three significant years (2024, 2017, 1991) which effectively cover different periods of Turkmenistan's holiday history.
🧰 Tools
🪛 Ruff (0.8.2)
9-9: Trailing whitespace
Remove trailing whitespace
(W291)
10-10: Trailing whitespace
Remove trailing whitespace
(W291)
12-29: Comprehensive test for fixed holidays.The test effectively verifies all fixed holidays for 2024 with a clean, efficient approach.
66-83: Be aware of hard-coded Islamic holiday dates.Testing specific dates for Islamic holidays might be fragile as these dates shift each year and depend on the calculation method.
The specific dates used (April 10, 2024 for Eid al-Fitr and June 16, 2024 for Eid al-Adha) should be verified against authoritative Islamic calendar sources for 2024. These dates can vary based on moon sighting and calculation methods.
The approach to test that there are at least 6 Eid days in total is robust and does not depend on specific dates.
🧰 Tools
🪛 Ruff (0.8.2)
72-72: Trailing whitespace
Remove trailing whitespace
(W291)
73-73: Trailing whitespace
Remove trailing whitespace
(W291)
74-74: Blank line contains whitespace
Remove whitespace from blank line
(W293)
77-77: Blank line contains whitespace
Remove whitespace from blank line
(W293)
80-80: Blank line contains whitespace
Remove whitespace from blank line
(W293)
81-81: Line too long (114 > 99)
(E501)
holidays/countries/turkmenistan.py
Outdated
| class TM(Turkmenistan): | ||
| pass | ||
|
|
||
| class TKM(Turkmenistan): | ||
| pass |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Standard alias classes follow project conventions.
The TM and TKM aliases align with ISO 3166-1 alpha-2 and alpha-3 codes.
Add a newline at the end of the file.
🧰 Tools
🪛 Ruff (0.8.2)
69-69: No newline at end of file
Add trailing newline
(W292)
holidays/countries/turkmenistan.py
Outdated
| """ | ||
|
|
||
| country = "TM" | ||
| start_year = 1991 |
There was a problem hiding this comment.
Maybe consider moving this to 1992 instead to avoid overlapping with USSR ones and simplify holidays assignment:
| start_year = 1991 | |
| start_year = 1992 |
holidays/countries/turkmenistan.py
Outdated
| """Add public holidays for Turkmenistan.""" | ||
| if self._year >= 1991: |
There was a problem hiding this comment.
You can safely remove the docstring for _populate_public_holidays.
Also, start_year = [YEAR] above already acts as if self._year >= [YEAR] for all holidays assignment.
holidays/countries/turkmenistan.py
Outdated
| References: | ||
| https://en.wikipedia.org/wiki/Public_holidays_in_Turkmenistan | ||
| https://www.timeanddate.com/holidays/turkmenistan/ | ||
| """ |
There was a problem hiding this comment.
Consider adding government sources as well (ru and tk translation is also available there - I would recommend making tk the default_language instead): https://www.mfa.gov.tm/en/articles/2, and based your holidays implementation from there
There was a problem hiding this comment.
Hey, i wrote localization test for three languages.(tk, en, ru). I just wanted to confirm: Is this code ok? or should I be creating the .po files for these languages myself, or will you be handling that part?
`
def test_localization(self):
Expected holidays here
try:
turkmen_holidays = Turkmenistan(years=range(1992, 2050), language="tk")
for holiday in turkmen_holidays:
self.assertEqual(holiday.lang, "tk", f"Holiday {holiday.name} not localized in Turkmen.")
self.assertEqual(holiday.name, expected_holidays_tk.get(holiday.name, holiday.name), f"Holiday name for {holiday.name} is not correctly localized in Turkmen.")
except FileNotFoundError:
print("Turkmen language translation files not found")
`
There was a problem hiding this comment.
I suggest you looking at the code of other countries, both in regards to holidays as well as tests.
tests/countries/test_turkmenistan.py
Outdated
| class TestTurkmenistanHolidays(unittest.TestCase): | ||
| def setUp(self): | ||
| """Initialize Turkmenistan holidays for testing.""" | ||
| self.holidays_2024 = Turkmenistan(years=2024) | ||
| self.holidays_2017 = Turkmenistan(years=2017) | ||
| self.holidays_1991 = Turkmenistan(years=1991) | ||
|
|
||
| def test_fixed_holidays_2024(self): |
There was a problem hiding this comment.
Consider looking up test_kazakhstan.py and test_macau.py for examples
|
You might want to take a look at our contributing guidelines for some helpful pointers. I've marked this PR as a draft for now, as it looks like it's still in the early stages of development. |
Still working on the project -- I'll get back to you with an update shortly. |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
holidays/countries/turkmenistan.py (2)
83-88: 🧹 Nitpick (assertive)Standard alias classes follow project conventions.
The TM and TKM aliases correctly implement the ISO 3166-1 alpha-2 and alpha-3 codes.
Remember to add a newline at the end of the file.
Add a newline at the end of the file:
class TKM(Turkmenistan): pass +🧰 Tools
🪛 Ruff (0.8.2)
88-88: No newline at end of file
Add trailing newline
(W292)
25-80: 🧹 Nitpick (assertive)Holiday implementation is historically accurate and well-structured.
The holiday definitions correctly handle:
- Fixed date holidays
- Date changes over time (e.g., Independence Day)
- Name changes (e.g., Revival Day → Constitution and Revival Day)
- The complex transitions of Neutrality Day
- Islamic holidays with their variable dates
However, there are several trailing whitespace issues that should be cleaned up.
Clean up trailing whitespace on lines 31, 41, 49, 55, 61, 65, and 73:
- # Memorial Day + # Memorial Day - # Nowruz + # Nowruz - # Constitution and Revival Day + # Constitution and Revival Day - # Independence Day + # Independence Day - # Day of Remembrance + # Day of Remembrance - # Neutrality Day + # Neutrality Day - # Islamic Holidays + # Islamic Holidays🧰 Tools
🪛 Ruff (0.8.2)
31-31: Trailing whitespace
Remove trailing whitespace
(W291)
41-41: Trailing whitespace
Remove trailing whitespace
(W291)
49-49: Trailing whitespace
Remove trailing whitespace
(W291)
55-55: Trailing whitespace
Remove trailing whitespace
(W291)
61-61: Trailing whitespace
Remove trailing whitespace
(W291)
65-65: Trailing whitespace
Remove trailing whitespace
(W291)
73-73: Trailing whitespace
Remove trailing whitespace
(W291)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/__init__.py(1 hunks)holidays/countries/turkmenistan.py(1 hunks)tests/countries/test_turkmenistan.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turkmenistan.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
🧬 Code Graph Analysis (2)
holidays/countries/__init__.py (1)
holidays/countries/turkmenistan.py (3)
Turkmenistan(6-80)TM(83-84)TKM(87-88)
holidays/countries/turkmenistan.py (3)
holidays/groups/international.py (4)
InternationalHolidays(18-220)_add_new_years_day(126-134)_add_new_years_day_two(136-144)_add_womens_day(187-197)holidays/groups/islamic.py (7)
IslamicHolidays(20-416)_add_eid_al_fitr_day(152-163)_add_eid_al_fitr_day_two(165-173)_add_eid_al_fitr_day_three(175-183)_add_eid_al_adha_day(109-120)_add_eid_al_adha_day_two(122-130)_add_eid_al_adha_day_three(132-140)holidays/holiday_base.py (1)
HolidayBase(57-1275)
🪛 Ruff (0.8.2)
holidays/countries/turkmenistan.py
31-31: Trailing whitespace
Remove trailing whitespace
(W291)
41-41: Trailing whitespace
Remove trailing whitespace
(W291)
49-49: Trailing whitespace
Remove trailing whitespace
(W291)
55-55: Trailing whitespace
Remove trailing whitespace
(W291)
61-61: Trailing whitespace
Remove trailing whitespace
(W291)
65-65: Trailing whitespace
Remove trailing whitespace
(W291)
73-73: Trailing whitespace
Remove trailing whitespace
(W291)
88-88: No newline at end of file
Add trailing newline
(W292)
tests/countries/test_turkmenistan.py
22-22: Line too long (109 > 99)
(E501)
32-32: Line too long (107 > 99)
(E501)
79-79: Line too long (105 > 99)
(E501)
81-81: print found
Remove print
(T201)
82-82: Blank line contains whitespace
Remove whitespace from blank line
(W293)
86-86: Line too long (105 > 99)
(E501)
88-88: print found
Remove print
(T201)
89-89: Blank line contains whitespace
Remove whitespace from blank line
(W293)
93-93: Line too long (108 > 99)
(E501)
95-95: print found
Remove print
(T201)
95-95: No newline at end of file
Add trailing newline
(W292)
🔇 Additional comments (13)
tests/countries/test_turkmenistan.py (8)
1-5: Good class structure following project conventions.The class correctly inherits from CommonCountryTests, WorkingDayTests, and TestCase, which aligns with the project's patterns.
6-9: Appropriate test setup with correct year range.The setUpClass method correctly initializes the test environment with years starting from 1992 (Turkmenistan's independence) through 2050, matching the implementation in the Turkmenistan class.
10-11: Simple and effective alias verification.This is a standard test to verify that country code aliases work correctly.
13-14: Good boundary testing for 1991.This effectively verifies that no holidays exist before the start_year (1992).
16-19: Efficient testing of New Year holidays.Using generator expressions to create date ranges is a clean approach for testing fixed-date holidays.
24-29: Good testing of conditional holiday.This correctly verifies both the presence of Defenders Day from 2009 and its absence before 2009.
40-65: Well-structured holiday tests with correct date transitions.The tests for Victory Day, Constitution and Revival Day, Independence Day, Day of Remembrance, and Neutrality Day effectively verify both the correct dates and the transitions between different names and dates.
67-73: Islamic holiday tests have appropriate estimated dates.The tests correctly verify the estimated dates for Eid al-Fitr and Eid al-Adha in 2024 and 2025, which is appropriate for movable Islamic holidays.
holidays/countries/__init__.py (1)
165-165: The Turkmenistan import looks good.The import for Turkmenistan is correctly positioned alphabetically between Turkey and Tuvalu, following the project's established pattern.
holidays/countries/turkmenistan.py (4)
1-5: Import statements are appropriate.The imports include only what's necessary for the implementation. Good work.
6-18: Well-structured class definition with proper documentation.The class is well designed with:
- Appropriate inheritance from base classes
- Good documentation with references
- Correct ISO country code "TM"
- Turkmen set as default language
- Start year of 1992 matching Turkmenistan's independence
- Support for appropriate languages (en_US, tk, ru)
20-23: Constructor properly initializes all parent classes.The initialization sequence is correct, ensuring all inherited functionality is available.
57-59:✅ Verification successful
Check spelling consistency in Independence Day holiday.
There's an inconsistency in the Turkmen spelling - line 57 uses "гүni" while line 59 uses "гүни". Verify which spelling is correct.
🏁 Script executed:
#!/bin/bash # Verify the spelling consistency in the Turkmen word "гүни" echo "Checking for word usage patterns in Turkmenistan implementation..." grep -n "гү[нn][иi]" holidays/countries/turkmenistan.pyLength of output: 200
Ensure Consistent Spelling for the Independence Day Holiday
It appears there may be a discrepancy in spelling on lines 57 and 59 of
holidays/countries/turkmenistan.py: one line uses “Гарашсызлык гүni” (with Latin letters for “ni”) while the other uses “Гарашсызлык гүни” (using fully Cyrillic characters). Please manually verify which version correctly reflects the Turkmen orthography and adjust the code accordingly.
🏁 Script executed:
#!/bin/bash # Print lines 55 to 65 to verify the exact spellings used for the Independence Day holiday. sed -n '55,65p' holidays/countries/turkmenistan.pyLength of output: 410
Independence Day Spelling is Consistent
After reviewing the code, both conditional paths now use the same spelling “Гарашсызлык гүни” for the Independence Day holiday. No further action is needed.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
tests/countries/test_turkmenistan.py (1)
120-120: 🧹 Nitpick (assertive)Add a newline at the end of the file.
Following good coding practices, add a trailing newline at the end of the file.
except FileNotFoundError: print("English language translation files not found") +holidays/countries/turkmenistan.py (1)
96-102: 🧹 Nitpick (assertive)Standard alias classes follow project conventions.
The TM and TKM aliases align with ISO 3166-1 alpha-2 and alpha-3 codes.
Add a newline at the end of the file.
class TKM(Turkmenistan): pass +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/turkmenistan.py(1 hunks)tests/countries/test_turkmenistan.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turkmenistan.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
🧬 Code Graph Analysis (1)
holidays/countries/turkmenistan.py (3)
holidays/groups/international.py (4)
InternationalHolidays(18-220)_add_new_years_day(126-134)_add_new_years_day_two(136-144)_add_womens_day(187-197)holidays/groups/islamic.py (7)
IslamicHolidays(20-416)_add_eid_al_fitr_day(152-163)_add_eid_al_fitr_day_two(165-173)_add_eid_al_fitr_day_three(175-183)_add_eid_al_adha_day(109-120)_add_eid_al_adha_day_two(122-130)_add_eid_al_adha_day_three(132-140)holidays/holiday_base.py (1)
HolidayBase(57-1275)
🪛 Ruff (0.8.2)
tests/countries/test_turkmenistan.py
101-101: print found
Remove print
(T201)
110-110: print found
Remove print
(T201)
119-119: print found
Remove print
(T201)
🔇 Additional comments (6)
tests/countries/test_turkmenistan.py (3)
19-22: Good implementation of test class structure.The test class correctly inherits from CommonCountryTests, WorkingDayTests, and TestCase, following project conventions. The setUpClass method is well implemented with the appropriate year range.
24-84: Well-structured holiday tests with appropriate year ranges.The holiday tests correctly verify:
- Country aliases
- Edge case testing for no holidays in 1991
- Specific holiday date changes (like Independence Day moving from Oct 27 to Sep 27)
- Name changes (like Revival Day becoming Constitution and Revival Day)
- Proper date ranges for holidays that started in specific years
This comprehensive testing approach ensures the accuracy of the holiday implementation.
85-91: Good inclusion of Islamic holiday tests.The tests for Eid al-Fitr and Eid al-Adha correctly verify the estimated dates for specific years (2024 and 2025), ensuring these movable holidays are properly implemented.
holidays/countries/turkmenistan.py (3)
19-31: Well-structured class definition with appropriate attributes.The Turkmenistan class extends the right base classes and includes proper documentation with relevant references. Setting the start year to 1992 is correct, as it aligns with Turkmenistan's independence and prevents overlap with USSR holidays.
33-36: Correct initialization of parent classes.The constructor properly initializes all parent classes in the right order.
38-94: Comprehensive implementation of Turkmenistan holidays with accurate historical transitions.Your implementation correctly handles various holiday date and name changes throughout Turkmenistan's history, including:
- Defender of the Fatherland Day (added in 2009)
- Constitution and Revival Day (name change in 2018)
- Independence Day (date change from Oct 27 to Sep 27 in 2018)
- Day of Remembrance (added in 2015)
- Neutrality Day's multiple transitions
The translations are properly integrated with the
tr()function, supporting the three languages specified in the class attributes.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Nabeel Ahmad <152245868+Nabeel-Ahmad07@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
tests/countries/test_turkmenistan.py (1)
107-107: 🧹 Nitpick (assertive)Add a newline at the end of the file.
Standard practice is to end files with a newline character. Add one to follow this convention.
self.skipTest(f"{lang_name} language translation files not found") +
♻️ Duplicate comments (1)
tests/countries/test_turkmenistan.py (1)
52-56: 🧹 Nitpick (assertive)Consider optimizing the Nowruz test implementation.
The current approach works but could be more concise by iterating over the days rather than having nested iterations.
def test_nowruz(self): name = "Наурыз мейрамы" - for year in range(1992, 2050): - self.assertHolidayName(name, f"{year}-03-21") - self.assertHolidayName(name, f"{year}-03-22") + for day in [21, 22]: + self.assertHolidayName(name, (f"{year}-03-{day}" for year in range(1992, 2050)))
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/countries/test_turkmenistan.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turkmenistan.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
🪛 Ruff (0.8.2)
tests/countries/test_turkmenistan.py
95-95: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🪛 GitHub Actions: CI/CD
tests/countries/test_turkmenistan.py
[error] 1-1: Trailing whitespace found. Please remove trailing whitespace from the file.
🔇 Additional comments (5)
tests/countries/test_turkmenistan.py (5)
19-23: Test structure follows project conventions properly.Good work implementing the test class with appropriate inheritance patterns and year ranges. The class correctly inherits from CommonCountryTests, WorkingDayTests, and TestCase, and initializes with a suitable year range.
24-29: Great coverage of basic holiday tests.The country alias test and no-holidays test for 1991 are correctly implemented. These fundamental tests ensure the core functionality works as expected.
40-46: Excellent implementation of year-based conditional testing.Your test for Defenders Day properly verifies both the existence of the holiday from 2009 onwards and its absence prior to 2009. This ensures the historical accuracy of the holiday implementation.
67-71: Good handling of Independence Day date change.The test correctly verifies the date change for Independence Day from October 27 to September 27 that occurred in 2018. This type of historical accuracy is important for holiday calculations.
77-83: Comprehensive testing of the complex Neutrality Day changes.Well done on implementing a thorough test of the Neutrality Day holiday, which has undergone multiple changes in both name and date over different time periods. This detailed approach ensures accurate holiday representation.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/countries/test_turkmenistan.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turkmenistan.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
🔇 Additional comments (14)
tests/countries/test_turkmenistan.py (14)
1-12: Good standard header with proper attribution.The header follows the project's conventions with comprehensive author attribution and MIT licensing information.
13-17: Great job following the project's import conventions.The imports follow the recommended pattern from previous reviews, using TestCase from unittest and CommonCountryTests from tests.common.
19-22: Excellent class structure and setup method.The TestTurkmenistan class correctly inherits from CommonCountryTests, WorkingDayTests, and TestCase. The setUpClass method properly calls its parent method and establishes the test years from 1992 to 2050.
24-28: Good basic test validations.The tests for country aliases and the empty 1991 year are concise and verify essential behavior.
30-33: Effective use of generator expressions for date ranges.Using generator expressions for testing holiday occurrences across multiple years is efficient and follows project conventions.
35-38: Well-implemented memorial day test.The test correctly verifies the memorial day holiday using proper name and date range.
40-46: Comprehensive defenders day test.Good job testing both the presence (2009-2050) and absence (1992-2009) of this holiday.
47-50: Well-implemented international women's day test.The test correctly verifies this holiday using the appropriate Kazakh name.
58-59: Concise victory day test implementation.Good use of generator expression for testing this holiday across all years.
61-71: Well-structured tests for holidays with changing names and dates.The tests properly verify Constitution Day and Independence Day across different periods with their name and date changes.
73-75: Good Day of Remembrance test.The test correctly verifies this holiday is only present from 2015 onward.
77-84: Thorough Neutrality Day test.The test effectively verifies the complex date and name changes for this holiday across different time periods.
85-91: Good Islamic holiday tests.The tests correctly verify Eid al-Fitr and Eid al-Adha for specific dates in 2024 and 2025.
93-107: Well-implemented localization test.The test efficiently handles multiple languages using a dictionary and proper exception handling with self.skipTest() instead of print statements.
|
|
I'm closing this due to a missed deadline. Feel free to reopen if you have a ready for review code. |
Proposed change
Add Turkmenistan holidays.
Fixes #1290
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green