Add Faroe Islands holidays #2606
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes introduce support for Faroe Islands holidays by adding a new country module, updating the country imports and registry, adding multiple localization files, and providing comprehensive tests. The Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate Unit Tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)holidays/countries/__init__.py(1 hunks)holidays/countries/faroe_islands.py(1 hunks)tests/countries/test_faroe_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_faroe_islands.py (2)
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: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/__init__.py (1)
holidays/countries/faroe_islands.py (3)
FaroeIslands(9-83)FO(86-87)FRO(90-91)
🪛 Ruff (0.11.9)
tests/countries/test_faroe_islands.py
3-3: holidays.constants.PUBLIC imported but unused
Remove unused import: holidays.constants.PUBLIC
(F401)
43-43: No newline at end of file
Add trailing newline
(W292)
holidays/countries/faroe_islands.py
1-1: datetime.date imported but unused
Remove unused import: datetime.date
(F401)
🪛 Pylint (3.3.7)
tests/countries/test_faroe_islands.py
[convention] 43-43: Final newline missing
(C0304)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 8-8: Missing class docstring
(C0115)
[warning] 10-10: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestFaroeIslands.setUpClass' method
(W0221)
[convention] 14-14: Missing function or method docstring
(C0116)
[convention] 18-18: Missing function or method docstring
(C0116)
[convention] 21-21: Missing function or method docstring
(C0116)
[convention] 24-24: Missing function or method docstring
(C0116)
[warning] 3-3: Unused PUBLIC imported from holidays.constants
(W0611)
holidays/countries/faroe_islands.py
[convention] 14-14: Line too long (112/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 86-86: Missing class docstring
(C0115)
[convention] 90-90: Missing class docstring
(C0115)
[warning] 1-1: Unused date imported from datetime
(W0611)
🔇 Additional comments (4)
.gitignore (1)
26-26: Good addition for Python cache exclusion.The recursive
**/__pycache__/pattern properly excludes all Python bytecode cache directories at any depth in the project structure.holidays/countries/__init__.py (1)
74-74: Properly integrated country import.The import follows the established pattern and is correctly positioned alphabetically among other country imports.
holidays/countries/faroe_islands.py (2)
9-83: Comprehensive and well-structured holiday implementation.The
FaroeIslandsclass properly inherits from the required base classes and implements a complete set of public holidays. The holiday definitions are well-documented with clear comments, and the use of the translation function ensures proper localization support. The start year of 1948 correctly corresponds to the year of home rule.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 14-14: Line too long (112/100)
(C0301)
86-91: Simple and effective alias classes.The
FOandFROalias classes provide the expected country code alternatives while maintaining the same functionality through inheritance.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 86-86: Missing class docstring
(C0115)
[convention] 90-90: Missing class docstring
(C0115)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/countries/test_faroe_islands.py (1)
44-44: 🧹 Nitpick (assertive)Add the missing trailing newline.
Despite previous attempts to fix this, the file still lacks a proper trailing newline.
Ensure the file ends with a single newline character after the closing parenthesis.
🧰 Tools
🪛 Ruff (0.11.9)
44-44: Blank line contains whitespace
Remove whitespace from blank line
(W293)
44-44: No newline at end of file
Add trailing newline
(W292)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.gitignore(1 hunks)tests/countries/test_faroe_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_faroe_islands.py (2)
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: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.11.9)
tests/countries/test_faroe_islands.py
42-42: Blank line contains whitespace
Remove whitespace from blank line
(W293)
44-44: Blank line contains whitespace
Remove whitespace from blank line
(W293)
44-44: No newline at end of file
Add trailing newline
(W292)
🪛 Pylint (3.3.7)
tests/countries/test_faroe_islands.py
[convention] 42-42: Trailing whitespace
(C0303)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 7-7: Missing class docstring
(C0115)
[warning] 9-9: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestFaroeIslands.setUpClass' method
(W0221)
[convention] 13-13: Missing function or method docstring
(C0116)
[convention] 17-17: Missing function or method docstring
(C0116)
[convention] 20-20: Missing function or method docstring
(C0116)
[convention] 23-23: Missing function or method docstring
(C0116)
🔇 Additional comments (4)
.gitignore (1)
26-26: Good addition for Python cache management.The recursive
**/__pycache__/pattern properly excludes Python bytecode cache directories at any depth, which is essential for clean version control.tests/countries/test_faroe_islands.py (3)
1-11: Solid test structure following project conventions.The class properly inherits from
CommonCountryTestsandTestCaseas recommended by project learnings, and the setup correctly initializes the years range.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 7-7: Missing class docstring
(C0115)
[warning] 9-9: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestFaroeIslands.setUpClass' method
(W0221)
13-14: Aliases test correctly implemented.Good coverage of the FO alias verification.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 13-13: Missing function or method docstring
(C0116)
17-21: Holiday name tests provide good coverage.Testing specific holidays like New Year's Day and National Day across the full year range ensures consistency.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 17-17: Missing function or method docstring
(C0116)
[convention] 20-20: Missing function or method docstring
(C0116)
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/faroe_islands.py(1 hunks)tests/countries/test_faroe_islands.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/countries/test_faroe_islands.py (2)
tests/common.py (4)
CommonCountryTests(356-374)assertAliases(121-130)assertHolidayName(195-199)assertLocalizedHolidays(327-338)holidays/countries/faroe_islands.py (2)
FaroeIslands(20-95)FO(98-99)
holidays/countries/faroe_islands.py (3)
holidays/groups/christian.py (11)
ChristianHolidays(22-463)_add_holy_thursday(328-337)_add_good_friday(308-317)_add_easter_sunday(270-279)_add_easter_monday(259-268)_add_ascension_thursday(119-127)_add_whit_sunday(443-453)_add_whit_monday(432-441)_add_christmas_eve(237-245)_add_christmas_day(208-216)_add_christmas_day_two(218-226)holidays/groups/international.py (3)
InternationalHolidays(18-220)_add_new_years_day(126-134)_add_new_years_eve(176-185)holidays/holiday_base.py (1)
HolidayBase(57-1301)
🪛 Pylint (3.3.7)
tests/countries/test_faroe_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 19-19: Missing class docstring
(C0115)
[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestFaroeIslands.setUpClass' method
(W0221)
[convention] 25-25: Missing function or method docstring
(C0116)
[convention] 28-28: Missing function or method docstring
(C0116)
[convention] 31-31: Missing function or method docstring
(C0116)
[convention] 34-34: Missing function or method docstring
(C0116)
holidays/countries/faroe_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 98-98: Missing class docstring
(C0115)
[convention] 102-102: Missing class docstring
(C0115)
🔇 Additional comments (3)
holidays/countries/faroe_islands.py (1)
20-95: Solid implementation of Faroe Islands holidays.The class structure is well-designed and follows the framework patterns correctly. The holiday implementation covers all major Faroese holidays with proper localization. The use of both Christian and International holiday groups is appropriate.
tests/countries/test_faroe_islands.py (2)
19-33: Comprehensive test coverage with proper structure.The test class correctly inherits from both
CommonCountryTestsandTestCase, following the framework patterns. The alias test and holiday name tests provide good coverage across multiple years.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 19-19: Missing class docstring
(C0115)
[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestFaroeIslands.setUpClass' method
(W0221)
[convention] 25-25: Missing function or method docstring
(C0116)
[convention] 28-28: Missing function or method docstring
(C0116)
[convention] 31-31: Missing function or method docstring
(C0116)
34-53: Thorough localization test covering all major holidays.The localization test comprehensively covers all the public holidays defined in the FaroeIslands class, ensuring the Faroese names are correctly mapped to their respective dates for 2023. This provides good validation of the holiday implementation.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 34-34: Missing function or method docstring
(C0116)
Thank you for guiding me. Please tell me how to implement a translation file in the locale folder. Do I have to create it for all the languages "da", "en_US", "fo", "is", "no", "sv". |
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (4)
holidays/locale/sv/LC_MESSAGES/FO.po (1)
29-102: 🧹 Nitpick (assertive)Verify Swedish translations.
All entries are populated, but please have a Swedish‐native double-check idiomatic accuracy for terms like “Bönedag”, “Grundlagsdag” and “Olofsmässafton”.holidays/locale/no/LC_MESSAGES/FO.po (1)
29-102: 🧹 Nitpick (assertive)Review Norwegian translations.
Allmsgstrfields are filled. Consider a quick native pass on entries like “Store bededag” and the observed-format string.holidays/locale/is/LC_MESSAGES/FO.po (1)
29-102: 🧹 Nitpick (assertive)Check Icelandic translations.
Entries look complete; recommend an Icelandic‐native review for terms like “Ólafsvöku aðfaranótt” and plural forms.holidays/locale/da/LC_MESSAGES/FO.po (1)
29-102: 🧹 Nitpick (assertive)Danish translations review.
Ensure consistency in capitalization (e.g., “Kristi Himmelfartsdag”) and that phrases like “Anden påskedag” follow Danish norms.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
README.md(2 hunks)holidays/locale/da/LC_MESSAGES/FO.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/FO.po(1 hunks)holidays/locale/fo/LC_MESSAGES/FO.po(1 hunks)holidays/locale/is/LC_MESSAGES/FO.po(1 hunks)holidays/locale/no/LC_MESSAGES/FO.po(1 hunks)holidays/locale/sv/LC_MESSAGES/FO.po(1 hunks)holidays/registry.py(1 hunks)
🔇 Additional comments (8)
holidays/locale/sv/LC_MESSAGES/FO.po (1)
15-28: PO header looks good.
Metadata andLanguage: svalign with project conventions. No changes required.holidays/locale/no/LC_MESSAGES/FO.po (1)
15-28: Confirm language code usage.
The header usesLanguage: no—verify this matches the project’s locale naming (e.g.,nbvs.nn).holidays/locale/en_US/LC_MESSAGES/FO.po (2)
15-28: PO header is correct.
Version, timestamps andLanguage: en_USmeet project standards.
29-102: English translations verified.
Translations accurately reflect official holiday names (e.g., “National Flag Day”, “St. Olaf’s Eve”).holidays/locale/is/LC_MESSAGES/FO.po (1)
15-28: PO header is fine.
Language: isis correct for Icelandic, and metadata follows conventions.holidays/locale/da/LC_MESSAGES/FO.po (1)
15-28: PO header consistency.
Metadata andLanguage: damatch expectations.holidays/registry.py (1)
83-83: Registration looks correct
Entry is properly named, in alphabetical order, and matches the class and code conventions.README.md (1)
108-108:✅ Verification successful
Validate country count header
The header says "184 country codes"; please ensure this matches the actual number of entries inCOUNTRIES.
🏁 Script executed:
#!/bin/bash # Verify that COUNTRY count equals 184 python3 - << 'EOF' from holidays.registry import COUNTRIES print(len(COUNTRIES)) EOFLength of output: 15
✅ Country count verified
The README header “184 country codes” matches the actual count (len(COUNTRIES) == 184). No changes required.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2606 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 253 254 +1
Lines 15201 15239 +38
Branches 2095 2095
=========================================
+ Hits 15201 15239 +38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Usual way is run |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
holidays/locale/fo/LC_MESSAGES/FO.po (2)
13-13: Clarify the localization comment.
Update the comment from
# Faroe Islands holidays fo localization.
to
# Faroe Islands holidays Faroese localization.
26-26: Remove unnecessary metadata.
Please delete the header line
"Generated-By: ChatGPT\n"
as it isn’t needed and may confuse future contributors.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)holidays/locale/fo/LC_MESSAGES/FO.po(1 hunks)
🔇 Additional comments (1)
holidays/locale/fo/LC_MESSAGES/FO.po (1)
31-32: Faroese suffix translation is correct.
Themsgstruses%s (eygleitt), which properly localizes the “(observed)” suffix. No change needed.
holidays/countries/faroe_islands.py
Outdated
| """ | ||
| Faroe Islands holidays. | ||
|
|
||
| References: | ||
| - https://visitfaroeislands.com/en/plan-your-stay/before-you-arrive-in-the-faroe-islands/\ | ||
| public-holidays | ||
| - https://en.wikipedia.org/wiki/Public_holidays_in_the_Faroe_Islands |
There was a problem hiding this comment.
| """ | |
| Faroe Islands holidays. | |
| References: | |
| - https://visitfaroeislands.com/en/plan-your-stay/before-you-arrive-in-the-faroe-islands/\ | |
| public-holidays | |
| - https://en.wikipedia.org/wiki/Public_holidays_in_the_Faroe_Islands | |
| """Faroe Islands holidays. | |
| References: | |
| * <https://en.wikipedia.org/wiki/Public_holidays_in_the_Faroe_Islands> | |
| * <https://visitfaroeislands.com/en/plan-your-stay/before-you-arrive-in-the-faroe-islands/public-holidays> |
You also need to add localization sources.
There was a problem hiding this comment.
Please elaborate. What exactly do you mean by localization sources?
There was a problem hiding this comment.
Where do you get translations into other languages?
There was a problem hiding this comment.
This question is still relevant. 👆
There was a problem hiding this comment.
Honestly speaking I used gemini + chatgpt
There was a problem hiding this comment.
We don't use such things. You can use Denmark holidays for checking the languages that are available there.
There was a problem hiding this comment.
- For Icelandic : https://en.wikipedia.org/wiki/Public_holidays_in_Iceland and I used google translate for other holidays which are not part of iceland. And I also referred other po files in our repo.
- For Swedish, Danish, Norweign - https://dictionary.cambridge.org/translate/ (Official Cambridge website)
- For Faroes Islands - https://en.wikipedia.org/wiki/Public_holidays_in_the_Faroe_Islands
- I also checked other po files of these languages and validated the translation.
So, can I add the the wiki links of each language translations and the these links as localization resources?
There was a problem hiding this comment.
Conclusion: The translations provided are perfect and non-erronous.
Please verify it if needed @KJhellico
There was a problem hiding this comment.
@KJhellico Any suggestions?
holidays/countries/faroe_islands.py
Outdated
|
|
||
| country = "FO" | ||
| default_language = "fo" | ||
| supported_categories = (OPTIONAL, PUBLIC) |
There was a problem hiding this comment.
Optional holidays should be placed into _populate_optional_holidays(), if any.
There was a problem hiding this comment.
I have not found any. Just kept it for future uses
There was a problem hiding this comment.
In Denmark Workers' Day, Constitution Day, Christmas Eve and New Year's Eve are OPTIONAL holidays. Is it different in Faroe Islands?
There was a problem hiding this comment.
No, I thoroughly checked, and all are either public holidays or half days.
holidays/locale/da/LC_MESSAGES/FO.po
Outdated
| "MIME-Version: 1.0\n" | ||
| "Content-Type: text/plain; charset=UTF-8\n" | ||
| "Content-Transfer-Encoding: 8bit\n" | ||
| "Generated-By: ChatGPT\n" |
There was a problem hiding this comment.
| "Generated-By: ChatGPT\n" |
There was a problem hiding this comment.
What source should I use? Please provide steps too.
|
First of all, you need to merge latest changes from |
Signed-off-by: Shrot101 <128983465+Shrot101@users.noreply.github.com>
These statements are not just a formality here. |
Yes, just found that. I just started so learning it. |
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
holidays/countries/__init__.py (1)
248-248: 🛠️ Refactor suggestionAdd missing Faroe Islands import to existing structure.
The Faroe Islands import is missing from the existing import structure. Add it in alphabetical order after Ethiopia.
from holidays.countries.ethiopia import Ethiopia, ET, ETH +from holidays.countries.faroe_islands import FaroeIslands, FO, FRO from holidays.countries.fiji import Fiji, FJ, FJItests/countries/test_faroe_islands.py (1)
61-62: 🧹 Nitpick (assertive)Add missing trailing newline.
The file is missing a trailing newline, which violates formatting standards.
self.assertLocalizedHolidays(*self.holidays_2023) +🧰 Tools
🪛 Pylint (3.3.7)
[convention] 61-61: Trailing newlines
(C0305)
♻️ Duplicate comments (2)
tests/countries/test_faroe_islands.py (2)
1-12: 🧹 Nitpick (assertive)Add missing module docstring.
Following project documentation standards, the test module needs a docstring.
+"""Tests for Faroe Islands holidays.""" + # holidays🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
58-60: 🛠️ Refactor suggestionRemove duplicate test logic.
The
test_2023_and_localizationmethod duplicates the exact same holiday verification astest_2023. Combine the localization testing into the existing test method.- def test_2023_and_localization(self): - self.assertHolidays(FaroeIslands(categories=(PUBLIC, HALF_DAY)), *self.holidays_2023) - self.assertLocalizedHolidays(*self.holidays_2023)And update the
test_2023method:def test_2023(self): self.assertHolidays(FaroeIslands(categories=(PUBLIC, HALF_DAY)), *self.holidays_2023) + self.assertLocalizedHolidays(*self.holidays_2023)🧰 Tools
🪛 Pylint (3.3.7)
[convention] 58-58: Missing function or method docstring
(C0116)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)holidays/countries/__init__.py(2 hunks)holidays/registry.py(1 hunks)tests/countries/test_faroe_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
README.md (1)
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: README.md:562-568
Timestamp: 2025-06-06T16:02:09.866Z
Learning: The README.md country table displays ISO 3166-1 alpha-2 codes only in the "Code" column. Alpha-3 codes or country aliases should not be included in this table format, maintaining consistency with all other country entries.
tests/countries/test_faroe_islands.py (1)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:47:27.213Z
Learning: For holiday tests in the vacanza/holidays project, structure tests by individual holidays rather than by years. Each test method should focus on a specific holiday and test it across multiple years (from start_year through 2050) using helper methods like `assertHolidayName`. For fixed holidays, use generators like `(f"{year}-01-01" for year in range(1991, 2051))`. For movable holidays, specify individual dates for specific years followed by a range check.
🧬 Code Graph Analysis (1)
holidays/countries/__init__.py (7)
holidays/countries/american_samoa.py (4)
AmericanSamoa(50-51)AS(42-43)ASM(46-47)HolidaysAS(17-39)holidays/countries/faroe_islands.py (3)
FaroeIslands(20-95)FO(98-99)FRO(102-103)holidays/countries/french_southern_territories.py (4)
FrenchSouthernTerritories(40-41)TF(44-45)ATF(48-49)HolidaysTF(16-37)holidays/countries/guam.py (4)
Guam(50-51)GU(42-43)GUM(46-47)HolidaysGU(17-39)holidays/countries/northern_mariana_islands.py (4)
NorthernMarianaIslands(50-51)MP(42-43)MNP(46-47)HolidaysMP(17-39)holidays/countries/puerto_rico.py (4)
PuertoRico(50-51)PR(42-43)PRI(46-47)HolidaysPR(17-39)holidays/countries/united_states.py (3)
UnitedStates(34-1242)US(1245-1246)USA(1249-1250)
🪛 Pylint (3.3.7)
holidays/countries/__init__.py
[error] 189-189: Parsing failed: ''(' was never closed (holidays.countries.init, line 189)'
(E0001)
tests/countries/test_faroe_islands.py
[convention] 61-61: Trailing newlines
(C0305)
[convention] 1-1: Missing module docstring
(C0114)
[error] 16-16: No name 'countries' in module 'holidays'
(E0611)
[convention] 20-20: Missing class docstring
(C0115)
[warning] 22-22: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestFaroeIslands.setUpClass' method
(W0221)
[convention] 46-46: Missing function or method docstring
(C0116)
[convention] 49-49: Missing function or method docstring
(C0116)
[convention] 52-52: Missing function or method docstring
(C0116)
[convention] 55-55: Missing function or method docstring
(C0116)
[convention] 58-58: Missing function or method docstring
(C0116)
🔇 Additional comments (1)
holidays/registry.py (1)
83-83: Solid Faroe Islands entry!
Ensure the correspondingholidays.countries.faroe_islandsmodule is imported inholidays/countries/__init__.pyso the lazy loader can find it.
|
@KJhellico, I updated the code. Please check it. I tried to resolve the issues. If there are any further issues, please guide me. Thank you for guiding me until now. I am learning a lot from this repo and you. |
holidays/countries/faroe_islands.py
Outdated
| # Christmas Day. | ||
| self._add_christmas_day(tr("Jóladagur")) | ||
|
|
||
| # Boxing Day. |
There was a problem hiding this comment.
| # Boxing Day. | |
| # Second Day of Christmas. |
There was a problem hiding this comment.
It's also en_US translation.
Signed-off-by: Shrot101 <128983465+Shrot101@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
holidays/countries/faroe_islands.py(1 hunks)holidays/locale/da/LC_MESSAGES/FO.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/FO.po(1 hunks)holidays/locale/fo/LC_MESSAGES/FO.po(1 hunks)holidays/locale/is/LC_MESSAGES/FO.po(1 hunks)holidays/locale/no/LC_MESSAGES/FO.po(1 hunks)holidays/locale/sv/LC_MESSAGES/FO.po(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
holidays/locale/da/LC_MESSAGES/FO.po (1)
Learnt from: PPsyrius
PR: vacanza/holidays#2608
File: holidays/locale/en_VC/LC_MESSAGES/VC.po:61-66
Timestamp: 2025-06-10T05:07:29.372Z
Learning: For missing translator comments in .po localization files in the holidays repository, direct authors to run `make l10n` or `make check` commands instead of suggesting manual fixes, as these commands automatically handle translator comment generation.
holidays/locale/fo/LC_MESSAGES/FO.po (2)
Learnt from: KJhellico
PR: vacanza/holidays#2259
File: holidays/locale/en_IN/LC_MESSAGES/IN.po:30-299
Timestamp: 2025-03-05T17:51:00.633Z
Learning: In the Holidays project, .po files for a country's default locale use empty msgstr fields as a standard convention.
Learnt from: PPsyrius
PR: vacanza/holidays#2438
File: holidays/locale/ar_IQ/LC_MESSAGES/IQ.po:35-81
Timestamp: 2025-04-17T17:08:48.082Z
Learning: In holiday PO files, when the file represents the default language of an entity (e.g., ar_IQ for Iraq), no translations in `msgstr` are required as the `msgid` values are already in the target language.
holidays/countries/faroe_islands.py (6)
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: holidays/countries/faroe_islands.py:0-0
Timestamp: 2025-06-06T13:23:16.229Z
Learning: Never suggest breaking long URLs in code comments or documentation. If something passes the pre-commit check, then it's allowed and should not be flagged for line length violations.
Learnt from: KJhellico
PR: vacanza/holidays#2609
File: holidays/countries/nauru.py:149-154
Timestamp: 2025-06-09T19:50:56.039Z
Learning: In the holidays library project, never suggest adding docstrings for alias classes (like NR, NRU country code aliases that inherit from main country classes). The project deliberately omits docstrings for these simple alias classes.
Learnt from: KJhellico
PR: vacanza/holidays#2571
File: holidays/countries/bermuda.py:108-110
Timestamp: 2025-05-31T15:11:42.151Z
Learning: In the holidays library project, alias classes (like BM, BMU) do not have docstrings. This is a project-specific convention.
Learnt from: KJhellico
PR: vacanza/holidays#2608
File: holidays/countries/saint_vincent_and_the_grenadines.py:1-12
Timestamp: 2025-06-08T13:36:16.999Z
Learning: For the holidays project/repository, module docstrings are not used and should never be suggested, even when pylint flags missing module docstring warnings (C0114).
Learnt from: KJhellico
PR: vacanza/holidays#2583
File: holidays/countries/niger.py:1-1
Timestamp: 2025-06-01T17:58:53.279Z
Learning: In the holidays project, module-level docstrings are not required or needed for country holiday files in the holidays/countries/ directory.
Learnt from: PPsyrius
PR: vacanza/holidays#2608
File: holidays/locale/en_VC/LC_MESSAGES/VC.po:61-66
Timestamp: 2025-06-10T05:07:29.372Z
Learning: For missing translator comments in .po localization files in the holidays repository, direct authors to run `make l10n` or `make check` commands instead of suggesting manual fixes, as these commands automatically handle translator comment generation.
🧬 Code Graph Analysis (1)
holidays/countries/faroe_islands.py (3)
holidays/groups/christian.py (10)
_add_holy_thursday(328-337)_add_good_friday(308-317)_add_easter_sunday(270-279)_add_easter_monday(259-268)_add_ascension_thursday(119-127)_add_whit_sunday(443-453)_add_whit_monday(432-441)_add_christmas_eve(237-245)_add_christmas_day(208-216)_add_christmas_day_two(218-226)holidays/groups/international.py (2)
_add_new_years_day(126-134)_add_new_years_eve(176-185)holidays/holiday_base.py (1)
HolidayBase(57-1301)
🪛 Pylint (3.3.7)
holidays/countries/faroe_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 100-100: Missing class docstring
(C0115)
[convention] 104-104: Missing class docstring
(C0115)
🔇 Additional comments (3)
holidays/countries/faroe_islands.py (1)
38-38:✅ Verification successful
Ensure all supported languages have .po files
The class lists"fo"and"no"amongsupported_languages, but I don’t see correspondingFO.pounder those locale folders. Please confirm that.pofiles exist forfoandno.
🏁 Script executed:
#!/bin/bash # Verify presence of FO.po for each supported locale for lang in da en_US fo is no sv; do if [ ! -f "holidays/locale/$lang/LC_MESSAGES/FO.po" ]; then echo "Missing FO.po for locale: $lang" exit 1 fi done echo "All supported FO.po files are present."Length of output: 623
FO.po translation files verified for all supported languages
A quick script check confirms thatholidays/locale/<lang>/LC_MESSAGES/FO.poexists forda,en_US,fo,is,no, andsv. No missing files detected—no further action needed.holidays/locale/no/LC_MESSAGES/FO.po (1)
13-14:⚠️ Potential issueHeader language tags are incorrect.
Language: foclaims the file is Faroese, but this is the Norwegian translation.
Update the header and the leading comment accordingly.-# Faroe Islands holidays no localization. +# Faroe Islands holidays Norwegian localization. @@ -"Language: fo\n" +"Language: no\n"Also applies to: 21-23
⛔ Skipped due to learnings
Learnt from: PPsyrius PR: vacanza/holidays#2437 File: holidays/locale/dz_BT/LC_MESSAGES/BT.po:0-0 Timestamp: 2025-04-06T14:52:35.679Z Learning: When suggesting changes to .po files in the holidays project, follow the standardized header format: - Default language format: "# [COUNTRY-NAME-NORMAL] holidays. #" - Non-default language format: "# [COUNTRY-NAME-NORMAL] holidays [LANGUAGE-CODE] localization. #" For example, for Bhutan in Dzongkha, use "# Bhutan holidays dz_BT localization. #"Learnt from: PPsyrius PR: vacanza/holidays#2437 File: holidays/locale/dz_BT/LC_MESSAGES/BT.po:12-12 Timestamp: 2025-04-06T14:44:59.785Z Learning: Use standardized .po file headers for localization files in the holidays project: - Default language format: "# [COUNTRY-NAME-NORMAL] holidays. #" - Other languages format: "# [COUNTRY-NAME-NORMAL] holidays [LANGUAGE-CODE] localization. #"holidays/locale/fo/LC_MESSAGES/FO.po (1)
29-96: Default-language PO looks good.
msgstrentries are correctly left blank, matching project conventions for default Faroese.
No further issues spotted.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
README.md (1)
108-108: 💡 Verification agent🧩 Analysis chain
Verify the “188 country codes” figure
Every time we add or remove a row the headline number drifts.
Please run the small script below – it simply counts<td>XX</td>occurrences – and bump the value if the count differs.
🏁 Script executed:
#!/bin/bash # Count country-code cells (two-letter codes) in the table. grep -oE '<td>[A-Z]{2}</td>' README.md | wc -lLength of output: 52
Update country code count to 189
The script reports 189
<td>XX</td>entries, so the “188” figure is out of date.• README.md (around line 108): update “188 country codes” → “189 country codes”
Suggested diff:
- We currently support 188 country codes. The standard way to refer to a country is by using its [ISO + We currently support 189 country codes. The standard way to refer to a country is by using its [ISO
♻️ Duplicate comments (1)
README.md (1)
562-568: 🧹 Nitpick (assertive)Indentation is inconsistent with neighbouring rows
Other table rows keep
<td>flush with the<tr>tag; this one is indented by two spaces, which stands out in diff-based docs.Quick tidy-up:
- <td>Faroe Islands</td> - <td>FO</td> - <td></td> - <td>da, en_US, <strong>fo</strong>, is, no, sv</td> - <td>HALF_DAY</td> +<td>Faroe Islands</td> +<td>FO</td> +<td></td> +<td>da, en_US, <strong>fo</strong>, is, no, sv</td> +<td>HALF_DAY</td>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)README.md(1 hunks)holidays/countries/__init__.py(1 hunks)holidays/registry.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
README.md (1)
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: README.md:562-568
Timestamp: 2025-06-06T16:02:09.910Z
Learning: The README.md country table displays ISO 3166-1 alpha-2 codes only in the "Code" column. Alpha-3 codes or country aliases should not be included in this table format, maintaining consistency with all other country entries.
🧬 Code Graph Analysis (1)
holidays/countries/__init__.py (1)
holidays/countries/faroe_islands.py (3)
FaroeIslands(20-97)FO(100-101)FRO(104-105)
🔇 Additional comments (1)
holidays/countries/__init__.py (1)
74-75: Faroe Islands import looks goodImport is alphabetically correct (between Ethiopia and Fiji) and mirrors the new registry entry. Nothing else to flag.
| @@ -24,3 +24,4 @@ Pipfile | |||
| sbom.json | |||
| site/ | |||
| venv/ | |||
| **/__pycache__/ | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
__pycache__ rule is fine but partly redundant
*.pyc (line 13) already blocks the compiled byte-code; adding **/__pycache__/ hides the directory too, which some teams prefer.
If the repo already has __pycache__/ at project root (common pattern in Python‐centric .gitignore templates) this new glob is a duplicate. Up to you, but a single rule is usually enough.
🤖 Prompt for AI Agents
In the .gitignore file at line 27, the rule to ignore all __pycache__
directories is redundant because the existing *.pyc rule at line 13 already
ignores compiled Python bytecode files. To fix this, decide whether to keep only
the *.pyc rule or the __pycache__ directory rule for simplicity, and remove the
redundant one accordingly.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
holidays/locale/no/LC_MESSAGES/FO.po (1)
26-26: Drop tool-specific metadata.
TheGenerated-By: Lingva 5.0.6header is unnecessary noise; please remove it.- "Generated-By: Lingva 5.0.6\n"holidays/locale/fo/LC_MESSAGES/FO.po (1)
26-26: Drop tool-specific metadata.
TheGenerated-By: Lingva 5.0.6header clutters the file; remove it to align with repo standards.- "Generated-By: Lingva 5.0.6\n"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
README.md(2 hunks)holidays/locale/da/LC_MESSAGES/FO.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/FO.po(1 hunks)holidays/locale/fo/LC_MESSAGES/FO.po(1 hunks)holidays/locale/is/LC_MESSAGES/FO.po(1 hunks)holidays/locale/no/LC_MESSAGES/FO.po(1 hunks)holidays/locale/sv/LC_MESSAGES/FO.po(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
README.md (1)
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: README.md:562-568
Timestamp: 2025-06-06T16:02:09.910Z
Learning: The README.md country table displays ISO 3166-1 alpha-2 codes only in the "Code" column. Alpha-3 codes or country aliases should not be included in this table format, maintaining consistency with all other country entries.
holidays/locale/da/LC_MESSAGES/FO.po (1)
Learnt from: PPsyrius
PR: vacanza/holidays#2608
File: holidays/locale/en_VC/LC_MESSAGES/VC.po:61-66
Timestamp: 2025-06-10T05:07:29.372Z
Learning: For missing translator comments in .po localization files in the holidays repository, direct authors to run `make l10n` or `make check` commands instead of suggesting manual fixes, as these commands automatically handle translator comment generation.
holidays/locale/fo/LC_MESSAGES/FO.po (2)
Learnt from: KJhellico
PR: vacanza/holidays#2259
File: holidays/locale/en_IN/LC_MESSAGES/IN.po:30-299
Timestamp: 2025-03-05T17:51:00.633Z
Learning: In the Holidays project, .po files for a country's default locale use empty msgstr fields as a standard convention.
Learnt from: PPsyrius
PR: vacanza/holidays#2438
File: holidays/locale/ar_IQ/LC_MESSAGES/IQ.po:35-81
Timestamp: 2025-04-17T17:08:48.082Z
Learning: In holiday PO files, when the file represents the default language of an entity (e.g., ar_IQ for Iraq), no translations in `msgstr` are required as the `msgid` values are already in the target language.
🔇 Additional comments (9)
holidays/locale/sv/LC_MESSAGES/FO.po (2)
1-28: PO file header and structure look solid.
TheLanguage: svheader is correct and the comment blocks match other locale files.
29-96: Swedish translations are spot on.
Allmsgstrentries accurately reflect the Faroese holiday names.holidays/locale/da/LC_MESSAGES/FO.po (2)
1-28: PO file header and structure look solid.
TheLanguage: daheader is correct; runmake l10nif you need to refresh translator comments automatically.
29-96: Danish translations are accurate.
Eachmsgstrmaps correctly to itsmsgid.holidays/locale/is/LC_MESSAGES/FO.po (2)
1-28: PO file header and structure look solid.
TheLanguage: isheader matches the file’s locale.
29-96: Icelandic translations are accurate.
Allmsgstrvalues correctly correspond to the sourcemsgidstrings.holidays/locale/en_US/LC_MESSAGES/FO.po (2)
1-28: PO file header and structure look solid.
TheLanguage: en_USheader is set correctly.
49-52: Great Prayer Day translation is now correct.
Themsgstrreads “Great Prayer Day” as intended.holidays/locale/no/LC_MESSAGES/FO.po (1)
13-13: 🧹 Nitpick (assertive)Clarify header comment.
The comment# Faroe Islands holidays no localization.could be confusing—replacenowithNorwegianfor clarity.- # Faroe Islands holidays no localization. + # Faroe Islands holidays Norwegian localization.⛔ Skipped due to learnings
Learnt from: ankushhKapoor PR: vacanza/holidays#2601 File: holidays/locale/en_MN/LC_MESSAGES/MN.po:13-14 Timestamp: 2025-06-11T18:32:25.595Z Learning: For non-default locale `.po` files, the header comment format is: `# <Country> holidays <locale> localization.` (no trailing hash).Learnt from: PPsyrius PR: vacanza/holidays#2437 File: holidays/locale/dz_BT/LC_MESSAGES/BT.po:0-0 Timestamp: 2025-04-06T14:52:35.679Z Learning: When suggesting changes to .po files in the holidays project, follow the standardized header format: - Default language format: "# [COUNTRY-NAME-NORMAL] holidays. #" - Non-default language format: "# [COUNTRY-NAME-NORMAL] holidays [LANGUAGE-CODE] localization. #" For example, for Bhutan in Dzongkha, use "# Bhutan holidays dz_BT localization. #"
| <tr> | ||
| <td>Faroe Islands</td> | ||
| <td>FO</td> | ||
| <td></td> | ||
| <td>da, en_US, <strong>fo</strong>, is, no, sv</td> | ||
| <td>HALF_DAY</td> | ||
| </tr> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Align indentation for the new row.
Remove the two-space prefix before each <td> in the Faroe Islands row so it lines up with other entries.
🤖 Prompt for AI Agents
In README.md around lines 562 to 568, the new table row for Faroe Islands has
two-space indentation before each <td> tag, which is inconsistent with other
rows. Remove the two-space prefix before each <td> tag in this row so that the
indentation aligns with the rest of the table entries.
There was a problem hiding this comment.
l10n suggestions are based on available translations from other countries and Norden info. Please feel free to make any comments.
| """ | ||
| Faroe Islands holidays. | ||
|
|
||
| References: | ||
| - https://visitfaroeislands.com/en/plan-your-stay/before-you-arrive-in-the-faroe-islands/\ | ||
| public-holidays | ||
| - https://guidetofaroeislands.fo/travel-information/faroe-islands-holiday/ | ||
| - https://www.timeanddate.com/holidays/faroe-islands/2025 | ||
| - https://www.framtak.com/info/holidays.html | ||
| - https://en.wikipedia.org/wiki/Public_holidays_in_Iceland | ||
| - https://dictionary.cambridge.org/translate/ | ||
| - https://en.wikipedia.org/wiki/Public_holidays_in_the_Faroe_Islands |
There was a problem hiding this comment.
| """ | |
| Faroe Islands holidays. | |
| References: | |
| - https://visitfaroeislands.com/en/plan-your-stay/before-you-arrive-in-the-faroe-islands/\ | |
| public-holidays | |
| - https://guidetofaroeislands.fo/travel-information/faroe-islands-holiday/ | |
| - https://www.timeanddate.com/holidays/faroe-islands/2025 | |
| - https://www.framtak.com/info/holidays.html | |
| - https://en.wikipedia.org/wiki/Public_holidays_in_Iceland | |
| - https://dictionary.cambridge.org/translate/ | |
| - https://en.wikipedia.org/wiki/Public_holidays_in_the_Faroe_Islands | |
| """Faroe Islands holidays. | |
| References: | |
| * <https://en.wikipedia.org/wiki/Public_holidays_in_the_Faroe_Islands> | |
| * [Decree no. 52 of 23 April 2003](https://web.archive.org/web/20241204233745/https://logir.fo/Kunngerd/52-fra-23-04-2003-um-flagging-fra-landsins-bygningum) | |
| * [2007(https://web.archive.org/web/20250125193236/https://www.framtak.com/info/holidays.html)) | |
| * [2024](https://web.archive.org/web/20250615193008/https://visitfaroeislands.com/en/plan-your-stay/get-ready-for-your-trip/general-facts/public-holidays) | |
| * [2025](https://web.archive.org/web/20250615193215/https://guidetofaroeislands.fo/travel-information/faroe-islands-holiday/) |
| default_language = "fo" | ||
| supported_categories = (HALF_DAY, PUBLIC) | ||
| supported_languages = ("da", "en_US", "fo", "is", "no", "sv") | ||
| start_year = 1948 # Year of home rule |
There was a problem hiding this comment.
| start_year = 1948 # Year of home rule | |
| # Denmark granted home rule to the Faroe Islands on 30 March 1948. | |
| start_year = 1949 |
| # National Flag Day. | ||
| self._add_holiday_apr_25(tr("Flaggdagur")) |
There was a problem hiding this comment.
National Flag Day should be placed before Constitution Day (in chronological order). And in all sources except Wikipedia, it's a full holiday, not half-day.
|
|
||
| #. Christmas Eve. | ||
| msgid "Jólaaftan" | ||
| msgstr "Juleaften" |
There was a problem hiding this comment.
| msgstr "Juleaften" | |
| msgstr "Juleaftensdag" |
holidays/locale/da/LC_MESSAGES/FO.po
Outdated
|
|
||
| #. Ascension Day. | ||
| msgid "Kristi himmalsferðardagur" | ||
| msgstr "Kristi Himmelfartsdag" |
There was a problem hiding this comment.
| msgstr "Kristi Himmelfartsdag" | |
| msgstr "Kristi himmelfartsdag" |
|
|
||
| #. Good Friday. | ||
| msgid "Langifríggjadagur" | ||
| msgstr "Långfredag" |
There was a problem hiding this comment.
| msgstr "Långfredag" | |
| msgstr "Långfredagen" |
|
|
||
| #. Great Prayer Day. | ||
| msgid "Dýri biðidagur" | ||
| msgstr "Bönedag" |
There was a problem hiding this comment.
| msgstr "Bönedag" | |
| msgstr "Stora bönedagen" |
|
|
||
| #. Saint Olaf's Day. | ||
| msgid "Ólavsøkudagur" | ||
| msgstr "Olofsmässodagen" |
There was a problem hiding this comment.
| msgstr "Olofsmässodagen" | |
| msgstr "Olafsdagen" |
|
|
||
| #. Saint Olaf's Eve. | ||
| msgid "Ólavsøkuaftan" | ||
| msgstr "Olofsmässafton" |
There was a problem hiding this comment.
| msgstr "Olofsmässafton" | |
| msgstr "Olafsafton" |
|
|
||
| #. National Flag Day. | ||
| msgid "Flaggdagur" | ||
| msgstr "Flaggdag" |
There was a problem hiding this comment.
| msgstr "Flaggdag" | |
| msgstr "Flaggdagen" |
|
|
Hi @Shrot101 |
I was outside due to some reasons. I will do it till tomorrow. |
|
Not completed by expected date. |
Proposed change
Type of change
holidaysfunctionality in general)Fixed: #2511
Checklist
make check, all checks and tests are greenReferences
https://visitfaroeislands.com/en/plan-your-stay/before-you-arrive-in-the-faroe-islands/public-holidays
https://en.wikipedia.org/wiki/Public_holidays_in_the_Faroe_Islands