Add Sao Tome and Principe holidays #2489
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update introduces official support for São Tomé and Príncipe holidays to the project. It adds a new country module, updates the registry and initialization files, and provides both English and Portuguese localizations for holiday names. The documentation and contributors list are updated accordingly. Comprehensive tests are included to verify holiday logic, observed rules, subdivisions, and localization for the new country. Changes
Suggested reviewers
✨ 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: 4
🔭 Outside diff range comments (3)
holidays/locale/pt/LC_MESSAGES/ST.po (1)
30-80: 🧹 Nitpick (assertive)Provide translations in msgstr for completeness. All
msgstrentries are currently empty, which may lead to empty outputs rather than falling back to the original strings. Consider populating them—at minimum mirroring themsgidvalues—to ensure proper display in the pt locale.tests/countries/test_sao_tome_and_principe.py (2)
156-166:⚠️ Potential issueFix variable scope in observed holidays test
There's a logic error in this test - the variable declaration for
jan1and subsequent code are outside the loop but reference the loop variableyear.def test_no_observed_holidays_before_2020(self): """Test that no observed holidays exist before 2020.""" for year in range(1975, 2020): # Before observed_start_year (2020) holidays = SaoTomeAndPrincipe(years=year, observed=True) - - # Check New Year's Day (Jan 1) never has an observed date before 2020 - jan1 = f"{year}-01-01" - if jan1 in holidays: - jan2 = f"{year}-01-02" - self.assertNotIn(jan2, holidays) # No observed date should exist + + # Check New Year's Day (Jan 1) never has an observed date before 2020 + jan1 = f"{year}-01-01" + if jan1 in holidays: + jan2 = f"{year}-01-02" + self.assertNotIn(jan2, holidays) # No observed date should exist
19-166: 💡 Verification agent🧩 Analysis chain
Consider restructuring tests by individual holidays
Based on project practices, consider restructuring tests to focus on individual holidays rather than years. For each holiday, test all relevant years in a single test method using helpers like
assertHolidayName.
🌐 Web query:
How should holiday test methods be structured in the vacanza/holidays project?💡 Result:
Structure of Holiday Test Methods in the vacanza/holidays Project
Overview
In the context of the vacanza/holidays project, "holiday test methods" likely refer to automated software tests that verify the correctness and behavior of holiday calculation functions, rather than physical testing of material coatings. While the search results focus on "holiday detection" in material sciences, best practices in structuring test methods for software holiday calculations can be inferred from established software testing conventions.
Recommended Structure for Holiday Test Methods
Organize by Region and Calendar System
- Separate tests for each country, region, or calendar supported by the project.
- Group similar types of holidays (e.g., public, religious, bank holidays) into dedicated test classes or files.
Use Descriptive Test Names
- Each test method should have a clear, descriptive name indicating which holiday or scenario it covers, such as
test_new_year,test_easter_date, ortest_substitute_holiday_on_weekend.Cover Key Holiday Scenarios
- Fixed-date holidays: Verify holidays that occur on the same date each year.
- Variable-date holidays: Test holidays based on rules (e.g., "first Monday in September", Easter).
- Substitution rules: Test scenarios where holidays falling on weekends are observed on another day.
- Edge cases: Handle leap years, daylight saving transitions, and historical changes to holiday definitions.
Parameterized Tests
- Use parameterization to test the same logic across multiple years and conditions, reducing repetition and increasing coverage.
Assertions
- Assert both presence (that a holiday is recognized on the correct date) and absence (that non-holiday dates are not incorrectly flagged).
- Test holiday names, dates, and any associated metadata (e.g., whether a holiday is official or observed).
Negative Tests
- Ensure that no false positives occur (e.g., non-holiday dates are not marked as holidays).
Data-Driven Approach
- Prefer external data sources or fixtures containing lists of expected holidays for given years, which tests can validate against.
Example (in Python-like pseudocode):
class TestUSAHolidays: def test_new_year(self): holiday = holidays.get('US', date(2025, 1, 1)) assert holiday.name == 'New Year\'s Day' def test_independence_day_substitution(self): # 4th of July 2026 is on a Saturday, substitution to Friday holiday = holidays.get('US', date(2026, 7, 3)) assert holiday.name == 'Independence Day (Observed)'
General Best Practices:
- Isolate test cases: Each method should test a single aspect or rule.
- Include regression tests: For any bugs found in the past, create a dedicated test to prevent recurrence.
- Document reasoning: Inline comments should clarify the purpose, especially for country-specific or complex holiday rules.
- Automate test execution: Integrate with continuous integration (CI) pipelines to ensure holiday logic is always up to date.
Summary Table
Structure Element Purpose Per-country & per-calendar Modular, maintainable codebase Descriptive test method names Readability and clarity Fixed & variable-date tests Comprehensive coverage Substitution & edge case tests Handles observed and special cases Parameterization & data-driven Efficient, scalable testing Negative & regression tests Reduces false positives, future-proofs logic
While the search results focused on physical "holiday testing" of coatings, these software test structuring guidelines are standard practice for holiday/date calculation libraries and should ensure robust, reliable holiday logic in the vacanza/holidays project.
Citations:
- 1: https://dot.ca.gov/-/media/dot-media/programs/engineering/documents/californiatestmethods-ctm/ctm-685-a11y.pdf
- 2: https://www.electrotechnicproducts.com/blog/holiday-testing-overview-and-links-to-standards-and-guides/
- 3: https://www.defelsko.com/resources/how-to-detect-holidays-in-pipelines-in-accordance-with-astm-g62
- 4: https://abtrex.com/holiday-detection/
- 5: https://www.madewell.net/images/stories/pdf/Holiday_Testing_TB.pdf
- 6: https://www.nde-solutions.com/hoilday-inspection/
- 7: https://www.usigroups.com/how-to/how-to-use-a-holiday-detector/
- 8: https://www.elcometerusa.com/Coating-Inspection/Porosity-Holiday-Detection/
- 9: https://www.korozyonuzmani.com/en/holidaygozenek-testi/
- 10: https://eur-lex.europa.eu/legal-content/ES/TXT/HTML/?uri=OJ%3AC%3A2014%3A331%3AFULL
Restructure tests per holiday using assertHolidayName
To align with vacanza/holidays conventions, each holiday should have its own test method covering all applicable years via the
assertHolidayNamehelper. This reduces boilerplate and makes it easier to maintain and extend.• Replace
test_consistent_holidayswith individual methods, for example:def test_ano_novo(self): # Fixed-date holiday: January 1 from start_year through 2050 self.assertHolidayName( (f"{year}-01-01" for year in range(2014, 2051)), "Ano Novo", )• Apply the same pattern to other fixed holidays (Dia dos Mártires, Dia do Trabalhador, etc.) using generators.
• For observed‑date rules, create separate methods, e.g.:def test_natal_observed(self): dates = [ "2020-12-24", # Natal (observed) when Dec 25 is Saturday "2022-12-26", # Natal (observed) when Dec 25 is Sunday # … ] self.assertHolidayName(dates, "Natal (observed)")• Move São Tomé Day, Príncipe‑specific and autonomy tests into their own
test_sao_tome_day,test_principe_descobrimento,test_autonomy_day_history, etc., each covering all relevant years.• Keep localization, suffix and negative‑case tests in their own methods (e.g.
test_observed_suffix_translation,test_no_observed_holidays_before_2020).This structure leverages
CommonCountryTestsutilities, makes each test’s purpose crystal‑clear, and follows established patterns in this repository.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
AUTHORS.md(1 hunks)README.md(2 hunks)holidays/countries/__init__.py(1 hunks)holidays/countries/sao_tome_and_principe.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/ST.po(1 hunks)holidays/locale/pt/LC_MESSAGES/ST.po(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_sao_tome_and_principe.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_sao_tome_and_principe.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 (2)
holidays/countries/__init__.py (1)
holidays/countries/sao_tome_and_principe.py (3)
SaoTomeAndPrincipe(24-130)ST(133-136)STP(139-142)
tests/countries/test_sao_tome_and_principe.py (2)
tests/common.py (2)
TestCase(28-338)CommonCountryTests(356-374)holidays/countries/sao_tome_and_principe.py (3)
SaoTomeAndPrincipe(24-130)ST(133-136)STP(139-142)
🔇 Additional comments (24)
holidays/registry.py (1)
155-157: Properly register São Tomé and Príncipe. Excellent: the newsao_tome_and_principeentry with class name and correct ISO codes is added in the right alphabetical slot betweensan_marinoandsaudi_arabia.holidays/countries/__init__.py (1)
147-149: Import new provider in package init. TheSaoTomeAndPrincipeclass and its aliasesST, STPare correctly imported betweenSanMarinoandSaudiArabia, matching registry ordering.README.md (1)
1076-1081: Document new country in README. The table row for São Tomé and Príncipe has been inserted before Saudi Arabia with correct subdivisions, supported languages (en_US, pt), and no additional categories.holidays/locale/en_US/LC_MESSAGES/ST.po (3)
31-33: LGTM! Correct observed suffix formatThe translation properly handles the observed holiday suffix pattern matching the implementation in the provider class.
35-69: LGTM! Comprehensive national holiday translationsWell-structured translations for all national holidays with appropriate comments explaining their historical context.
71-80: LGTM! Complete Príncipe subdivision holiday translationsAll subdivision-specific holidays are properly translated with contextual comments.
tests/countries/test_sao_tome_and_principe.py (9)
19-24: LGTM! Good test class setupThe test class properly extends CommonCountryTests and uses an appropriate year range (2014-2026) that covers the relevant period for holiday testing.
25-29: LGTM! Proper country alias verificationTest correctly verifies both country aliases (ST and STP) and their default subdivision settings.
31-51: LGTM! Good consistent holiday verificationThe test effectively verifies all consistent holidays across different years, ensuring core functionality works correctly.
[approve_code_checks]
52-82: LGTM! Thorough observed holidays validationComprehensive test cases covering both the absence of observed holidays before 2020 and their correct implementation from 2020 onwards.
83-99: LGTM! Accurate São Tomé Day historical testingTest correctly validates that São Tomé Day starts from 2019 and handles observed dates appropriately from 2020 onwards.
100-122: LGTM! Complete subdivision holiday testingGood verification of Príncipe-specific holidays being absent from national holiday sets but present when the subdivision is specified.
123-133: LGTM! Good localization testingTest properly verifies both Portuguese and English holiday name translations.
134-143: LGTM! Proper historical validationTest correctly verifies that Autonomy Day only appears from 1995 onwards in Príncipe.
145-155: LGTM! Thorough observed suffix translation testingTest verifies correct observed suffix handling across multiple languages.
holidays/countries/sao_tome_and_principe.py (9)
24-34: LGTM! Well-documented class with comprehensive referencesGood class definition with appropriate inheritance and well-structured docstring with references.
36-55: LGTM! Complete country defi 8000 nition with subdivisionsWell-structured country definition with all subdivisions and appropriate aliases.
57-62: LGTM! Good class attributes for localization and date rangesProper definition of language support, categories, and start years for holidays and observed dates.
64-75: LGTM! Well-implemented initialization with language-specific observed suffixGood initialization with proper inheritance chain and language-specific observed suffix handling.
76-80: LGTM! Good observed holiday handlingProper override of
_add_observedto respect the observed_start_year.
81-118: LGTM! Comprehensive national holidays implementationThorough implementation of all national holidays with proper comments and historical handling for São Tomé Day.
119-130: LGTM! Well-implemented subdivision-specific holidaysGood implementation of Príncipe-specific holidays with proper historical handling for Autonomy Day.
133-142: LGTM! Appropriate country aliasesGood implementation of alias classes with proper docstrings.
145-148: LGTM! Future-proof placeholder classGood placeholder class for potential future static holidays.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
holidays/locale/pt/LC_MESSAGES/ST.po (1)
31-81:⚠️ Potential issueFill in the empty msgstr fields.
All the Portuguese msgstr translations are currently empty, which means no translations will be shown. Since this is a Portuguese translation file for a Portuguese-speaking country, the msgstr values should match the msgid values.
Fill in each missing translation. For example:
msgid "Ano Novo" -msgstr "" +msgstr "Ano Novo"
♻️ Duplicate comments (3)
holidays/locale/pt/LC_MESSAGES/ST.po (1)
14-15: Update PO file header metadata.The placeholders
FIRST AUTHOR <EMAIL@ADDRESS>should be replaced with actual translator credentials, and thefuzzyflag should be removed if translations are ready for use.holidays/locale/en_US/LC_MESSAGES/ST.po (1)
23-23: Fix language setting in translation file headerThe language field in the header is set to "pt" (Portuguese) although this is an en_US translation file. This could cause issues with locale selection.
-"Language: pt\n" +"Language: en_US\n"holidays/countries/sao_tome_and_principe.py (1)
31-31: Fix typo in reference URLThere's a typo in the URL for timeanddate.com - extra 's' in "holidssays".
- * https://www.timeanddate.com/holidssays/sao-tome-and-principe/ + * https://www.timeanddate.com/holidays/sao-tome-and-principe/
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
holidays/countries/sao_tome_and_principe.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/ST.po(1 hunks)holidays/locale/pt/LC_MESSAGES/ST.po(1 hunks)tests/countries/test_sao_tome_and_principe.py(1 hunks)
🔇 Additional comments (2)
tests/countries/test_sao_tome_and_principe.py (1)
1-327: Well-structured and comprehensive test suite.This test file is thorough and covers all aspects of the implementation including:
- Country aliases and subdivisions
- Consistent holidays across years
- Observed holiday rules from 2020 onwards
- São Tomé Day introduction in 2019
- Príncipe-specific holidays
- Localization in multiple languages
- Edge cases and internal logic
The tests provide strong validation for the holiday provider implementation.
holidays/countries/sao_tome_and_principe.py (1)
24-130: Well-implemented holiday provider with comprehensive coverage.The implementation of the SaoTomeAndPrincipe class is well-structured and follows the package's conventions. It correctly:
- Handles observed holidays from 2020 onwards
- Includes São Tomé Day from 2019
- Provides subdivision support for Príncipe
- Implements language localization
- Follows existing patterns for holiday definitions
The class provides comprehensive holiday support for São Tomé and Príncipe with clear documentation.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
holidays/locale/en_US/LC_MESSAGES/ST.po (4)
14-15: 🧹 Nitpick (assertive)Remove fuzzy flag from finalized translations.
The
fuzzyflag indicates this is a draft translation that needs review. Remove this flag if the translations are final.
23-23: 8000⚠️ Potential issueFix language setting in translation file header.
The language field in the header is set to "pt" (Portuguese) although this is an en_US translation file. This could cause issues with locale selection.
-"Language: pt\n" +"Language: en_US\n"
79-81:⚠️ Potential issueAdd missing observed holiday suffix translation.
The observed holiday suffix format string has an empty msgstr. This will cause observed holidays to have no suffix in the UI.
#, c-format msgid "%s (observed)" -msgstr "" +msgstr "%s (observed)"
83-85: 🧹 Nitpick (assertive)Remove deprecated message entry.
This appears to be a deprecated or outdated message marked with
#~that's being replaced by the new format above. Since it's not needed, consider removing these lines.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
AUTHORS.md(1 hunks)holidays/countries/sao_tome_and_principe.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/ST.po(1 hunks)
🔇 Additional comments (3)
AUTHORS.md (1)
37-37: LGTM: Author added in alphabetical order.The new author "Devaraj K" has been correctly added in alphabetical order.
holidays/locale/en_US/LC_MESSAGES/ST.po (1)
19-20: 🧹 Nitpick (assertive)Update date references in header.
The dates in the header are set to 2025, which is in the future. Consider updating to the current year for accuracy.
-"POT-Creation-Date: 2025-04-22 01:37+0530\n" -"PO-Revision-Date: 2025-04-22 16:36+0530\n" +"POT-Creation-Date: 2024-04-22 01:37+0530\n" +"PO-Revision-Date: 2024-04-22 16:36+0530\n"Likely an incorrect or invalid review comment.
holidays/countries/sao_tome_and_principe.py (1)
1-148: Well-implemented holiday provider for São Tomé and Príncipe.The implementation is thorough, covering national holidays, subdivision-specific holidays, observed holiday rules, and proper localization support. The code structure follows the project's patterns and includes clear documentation.
|
IMO the overall implementation looks pretty good That said, I'd recommend using the standard test setup used in other countries' implementations - it helps keep things consistent and easier to maintain in the long run. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
holidays/countries/sao_tome_and_principe.py (7)
132-136: 🛠️ Refactor suggestionRemove docstrings from country code alias classes.
According to library convention for country code aliases (ISO 3166-1 alpha-2 and alpha-3 codes), these should be implemented as simple pass-through classes without docstrings.
class ST(SaoTomeAndPrincipe): - """Alias for São Tomé and Príncipe national holidays (no subdivision).""" - pass
90-92: 🧹 Nitpick (assertive)Comment structure for holiday names should be consistent.
Only the English name of the holiday should be in the comment for localization purposes.
- # Day of King Amador (January 4). - # Commemorates King Amador, leader of 16th century slave rebellion. + # Day of King Amador.Alternatively, if you want to preserve the historical context:
- # Day of King Amador (January 4). - # Commemorates King Amador, leader of 16th century slave rebellion. + # Day of King Amador (January 4). + # Commemorates King Amador, leader of 16th century slave rebellion. + + # Day of King Amador.
47-55: 🛠️ Refactor suggestionUpdate subdivision aliases to match the subdivision code.
The alias for "Principe" should map to "P" to match the subdivisions list, and Portuguese names should include proper diacritical marks.
subdivisions_aliases = { - "Agua Grande": "01", - "Cantagalo": "02", - "Caue": "03", - "Lemba": "04", - "Lobata": "05", - "Me-Zochi": "06", - "Principe": "PR", # Autonomous region + # Districts. + "Água Grande": "01", + "Cantagalo": "02", + "Caué": "03", + "Lembá": "04", + "Lobata": "05", + "Mé-Zóchi": "06", + # Autonomous Region. + "Príncipe": "P", }
138-141: 🛠️ Refactor suggestionRemove docstrings from country code alias classes.
Per library convention, alias classes should be implemented without docstrings.
class STP(SaoTomeAndPrincipe): - """Alias for São Tomé and Príncipe (allows subdivision specification).""" - pass
30-33: 🧹 Nitpick (assertive)Reference links should follow MkDocs formatting.
The reference URLs should be formatted with angle brackets for MkDocs compatibility.
- * https://en.wikipedia.org/wiki/Public_holidays_in_S%C3%A3o_Tom%C3%A9_and_Pr%C3%ADncipe - * https://www.timeanddate.com/holidays/sao-tome-and-principe/ - * https://www.saotomeexpert.pt/en/sao-tome-public-holidays/ - * https://www.qppstudio.net/publicholidays2025/sao_tome_and_principe.htm + * <https://en.wikipedia.org/wiki/Public_holidays_in_São_Tomé_and_Príncipe> + * <https://www.timeanddate.com/holidays/sao-tome-and-principe/> + * <https://www.saotomeexpert.pt/en/sao-tome-public-holidays/> + * <https://www.qppstudio.net/publicholidays2025/sao_tome_and_principe.htm>
37-45:⚠️ Potential issueInconsistency between subdivision code definitions and usage.
The Príncipe subdivision is defined as "PR" here but is referenced as "PR" in the
_populate_public_holidaysmethod. This needs to be consistent.subdivisions = ( "01", "02", "03", "04", "05", "06", - "PR", + "P", )
57-62: 🛠️ Refactor suggestionUpdate configuration settings for consistency with library conventions.
The default language should be "pt_ST" to follow the convention for ex-Portuguese territories. The
supported_categoriesdeclaration is unnecessary since PUBLIC is already the default.-default_language = "pt" -supported_categories = (PUBLIC,) -observed_label = tr("%s (observed)") -supported_languages = ("en_US", "pt") -start_year = 2014 -observed_start_year = 2020 # Year when observed holidays began +default_language = "pt_ST" +# %s (observed). +observed_label = tr("%s (observado)") +supported_languages = ("en_US", "pt_ST") +start_year = 2014 +observed_start_year = 2020 # Year when observed holidays began
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/sao_tome_and_principe.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/sao_tome_and_principe.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (4)
holidays/locale/en_US/LC_MESSAGES/ST.po (3)
22-22:⚠️ Potential issueFix language setting in translation file header.
The language field in the header is set to "pt" (Portuguese) although this is an en_US translation file.
-"Language: pt\n" +"Language: en_US\n"
78-80:⚠️ Potential issueAdd missing observed holiday suffix translation.
The observed holiday suffix format string has an empty msgstr. This will cause observed holidays to have no suffix in the UI.
#, c-format msgid "%s (observed)" -msgstr "" +msgstr "%s (observed)"
82-84: 🧹 Nitpick (assertive)Remove deprecated message entry.
This appears to be a deprecated or outdated message marked with
#~that's being replaced by the new format above.-#, c-format -#~ msgid "%s (observado)" -#~ msgstr "%s (observed)"holidays/countries/sao_tome_and_principe.py (1)
68-70: 🧹 Nitpick (assertive)Remove redundant docstring and implementation.
The docstring and the check for
self._year < self.start_yearis redundant since this functionality is already implemented in theHolidayBaseclass.- def _populate_public_holidays(self): - """Populate holidays - national for all, plus extras for Príncipe.""" - if self._year < self.start_year: - return None + def _populate_public_holidays(self):
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/sao_tome_and_principe.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/ST.po(1 hunks)holidays/locale/pt/LC_MESSAGES/ST.po(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/locale/pt/LC_MESSAGES/ST.po (2)
Learnt from: KJhellico
PR: vacanza/holidays#2394
File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88
Timestamp: 2025-03-31T19:37:57.691Z
Learning: In the holidays library localization pattern, when a locale file matches a country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language locale files should have translations in the msgstr fields.
Learnt from: KJhellico
PR: vacanza/holidays#2394
File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88
Timestamp: 2025-03-31T19:37:57.691Z
Learning: In the holidays library, when a locale file matches the country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language files should have filled msgstr fields with translations.
holidays/countries/sao_tome_and_principe.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
🪛 Ruff (0.8.2)
holidays/countries/sao_tome_and_principe.py
55-55: Trailing whitespace
Remove trailing whitespace
(W291)
61-61: Undefined name SaoTomeAndPrincipeStaticHolidays
(F821)
🔇 Additional comments (5)
holidays/locale/pt/LC_MESSAGES/ST.po (3)
29-31: Format string for observed holidays matches project pattern.The observed holiday format string is correctly left empty in the msgstr field, following the project's convention for default language files.
33-67: National holidays translated correctly with empty msgstr fields.The national holiday translations follow the project's convention for default language files by having empty msgstr fields.
69-79: Príncipe subdivision holidays implemented correctly.The regional holidays for the Príncipe autonomous region are correctly implemented with empty msgstr fields following the project's convention.
holidays/countries/sao_tome_and_principe.py (2)
104-113: Well-structured subdivision holiday implementation.The implementation of Príncipe-specific holidays is done correctly with appropriate translation markers and observed rules.
115-120: Alias classes correctly implemented.The country code aliases follow the project convention as simple pass-through classes without docstrings.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
holidays/locale/en_US/LC_MESSAGES/ST.po (3)
22-22:⚠️ Potential issueFix
Languageheader for English locale. It’s currently set toptbut should been_USin this English translation file:-"Language: pt\n" +"Language: en_US\n"
13-13: 🧹 Nitpick (assertive)Validate header country name formatting. Other
.poheaders use unaccented ISO-based names to match file-level conventions. Consider dropping diacritics here:-# São Tomé and Príncipe holidays en_US localization. +# Sao Tome and Principe holidays en_US localization.
79-81:⚠️ Potential issueAdd translation for observed suffix. The
msgstris blank, so observed holidays will lack a suffix. Update to:-msgstr "" +msgstr "%s (observed)"holidays/countries/sao_tome_and_principe.py (3)
55-55: 🧹 Nitpick (assertive)Trim trailing whitespace. Remove the extra space after
"P",to satisfy style rules:- "Príncipe": "P", + "Príncipe": "P",🧰 Tools
🪛 Ruff (0.8.2)
55-55: Trailing whitespace
Remove trailing whitespace
(W291)
61-61:⚠️ Potential issueFix undefined reference to
SaoTomeAndPrincipeStaticHolidays. The class is not defined. Either define it or remove theclsargument if you don’t have static holidays. For example:- StaticHolidays.__init__(self, cls=SaoTomeAndPrincipeStaticHolidays) + StaticHolidays.__init__(self)🧰 Tools
🪛 Ruff (0.8.2)
61-61: Undefined name
SaoTomeAndPrincipeStaticHolidays(F821)
88-90: 🧹 Nitpick (assertive)Simplify and consolidate comments. Keep only the holiday name; drop the extra context:
- # Agricultural Reform Day (September 30). - # Nationalization of plantations in 1975. + # Agricultural Reform Day.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)holidays/countries/sao_tome_and_principe.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/ST.po(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/locale/en_US/LC_MESSAGES/ST.po (1)
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: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. #"
holidays/countries/sao_tome_and_principe.py (4)
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:86-88
Timestamp: 2025-04-23T09:22:41.742Z
Learning: For holiday definitions in the holidays package, keep comments simple with just the holiday name (e.g., "# Independence Day.") rather than including dates or historical context, as the function names already encode the date information.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
🪛 Ruff (0.8.2)
holidays/countries/sao_tome_and_principe.py
55-55: Trailing whitespace
Remove trailing whitespace
(W291)
61-61: Undefined name SaoTomeAndPrincipeStaticHolidays
(F821)
🔇 Additional comments (2)
README.md (1)
102-102: Approve total count update. The supported country count has been correctly incremented from 169 to 170 to reflect the addition of São Tomé and Príncipe.holidays/locale/en_US/LC_MESSAGES/ST.po (1)
28-28: ConfirmX-Source-Language. The source language is correctlypt_ST(Portuguese, ST variant) since the originalmsgids are in Portuguese. No change needed.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
holidays/locale/pt_ST/LC_MESSAGES/ST.po (1)
33-80: 🧹 Nitpick (assertive)Add Portuguese translations
The msgstr fields are empty, meaning Portuguese translations are missing. While this works (as Portuguese is the source language), explicitly setting the translations improves clarity and maintainability.
Add translations by filling in the msgstr fields. For example:
msgid "Ano Novo" -msgstr "" +msgstr "Ano Novo"Do this for all holiday entries to ensure proper localization.
♻️ Duplicate comments (6)
holidays/locale/en_US/LC_MESSAGES/ST.po (1)
22-22:⚠️ Potential issueFix language setting in header
The language field is incorrectly set to "pt" instead of "en_US", which could cause issues with locale selection.
-"Language: pt\n" +"Language: en_US\n"tests/countries/test_sao_tome_and_principe.py (1)
19-25: 💡 Verification agent🧩 Analysis chain
Use recommended test structure
While the current approach works, consider adopting the standardized test setup used in other country implementations for consistency.
🏁 Script executed:
#!/bin/bash # Show examples of standardized test setups from other recent country implementations echo "Checking recent country test implementations for reference..." grep -A 15 "class Test" tests/countries/test_macau.py tests/countries/test_suriname.py | head -n 30Length of output: 2250
Use standardized test structure in TestSaoTomeAndPrincipe
To align with other country tests (e.g. Suriname, Macau), extract the common
yearsrange into a local variable and pass it through bothsuper().setUpClassand your holiday initializers:• File: tests/countries/test_sao_tome_and_principe.py
- Extract the years range into
years- Pass
years=yearstosuper().setUpClass- Reuse
yearsforcls.principe_holidaysProposed diff:
--- a/tests/countries/test_sao_tome_and_principe.py +++ b/tests/countries/test_sao_tome_and_principe.py @@ class TestSaoTomeAndPrincipe(CommonCountryTests, TestCase): - @classmethod - def setUpClass(cls): - super().setUpClass(SaoTomeAndPrincipe, years=range(2014, 2050)) - cls.observed_holidays = SaoTomeAndPrincipe(years=range(2020, 2050), observed=True) - cls.principe_holidays = SaoTomeAndPrincipe(subdiv="P", years=range(2014, 2050)) + @classmethod + def setUpClass(cls): + years = range(2014, 2050) + super().setUpClass(SaoTomeAndPrincipe, years=years) + cls.observed_holidays = SaoTomeAndPrincipe(years=range(2020, 2050), observed=True) + cls.principe_holidays = SaoTomeAndPrincipe(subdiv="P", years=years)holidays/countries/sao_tome_and_principe.py (4)
60-63:⚠️ Potential issueAlign observed_since with PR objectives.
The PR summary specifies that observed‑holiday logic applies starting in 2014, but the code uses 2020. Please update to 2014 and adjust the inline comment accordingly.
Proposed fix:
- # Based on common government practices since at least 2020. - kwargs.setdefault("observed_since", 2020) + # Based on common government practices since at least 2014. + kwargs.setdefault("observed_since", 2014)
76-77: 🧹 Nitpick (assertive)Simplify holiday comment.
For proper translation extraction, comments should contain only the holiday name—remove the date portion here.
Proposed fix:
- # Labor Day (May 1). + # Labor Day.
82-83: 🧹 Nitpick (assertive)Simplify holiday comment.
Remove the date from this line to keep the naming comments consistent across entries.
Proposed fix:
- # Armed Forces Day (September 6). + # Armed Forces Day.
85-88: 🧹 Nitpick (assertive)Simplify holiday comment and separate context.
Only the holiday name should appear on the primary comment line. Move the historical context into a secondary comment if you want to preserve it.
Proposed fix:
- # Agricultural Reform Day (September 30). - # Nationalization of plantations in 1975. + # Agricultural Reform Day. + # Commemorates nationalization of plantations in 1975 (September 30).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)holidays/countries/sao_tome_and_principe.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/ST.po(1 hunks)holidays/locale/pt_ST/LC_MESSAGES/ST.po(1 hunks)tests/countries/test_sao_tome_and_principe.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
holidays/locale/en_US/LC_MESSAGES/ST.po (1)
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. #"
tests/countries/test_sao_tome_and_principe.py (4)
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.
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.
holidays/countries/sao_tome_and_principe.py (4)
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:86-88
Timestamp: 2025-04-23T09:22:41.742Z
Learning: For holiday definitions in the holidays package, keep comments simple with just the holiday name (e.g., "# Independence Day.") rather than including dates or historical context, as the function names already encode the date information.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
🧬 Code Graph Analysis (1)
tests/countries/test_sao_tome_and_principe.py (2)
tests/common.py (6)
TestCase(28-338)CommonCountryTests(356-374)assertHolidays(228-230)assertHoliday(150-152)assertNoHoliday(244-246)assertLocalizedHolidays(327-338)holidays/countries/sao_tome_and_principe.py (3)
SaoTomeAndPrincipe(19-104)ST(107-108)STP(111-112)
🔇 Additional comments (8)
README.md (2)
102-102: Good job updating the country count!The total supported country count is correctly updated from 169 to 170 with the addition of São Tomé and Príncipe.
1076-1081: Country information looks complete and consistent.The entry for São Tomé and Príncipe includes all required information and follows the established format. The subdivision codes and supported languages are correctly specified.
holidays/locale/pt_ST/LC_MESSAGES/ST.po (1)
29-31: 🛠️ Refactor suggestionImplement observed holiday suffix translation
The observed holiday suffix format string has an empty msgstr. This could cause observed holidays to have no proper suffix in Portuguese.
#, c-format msgid "%s (observado)" -msgstr "" +msgstr "%s (observado)"⛔ Skipped due to learnings
Learnt from: KJhellico PR: vacanza/holidays#2394 File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88 Timestamp: 2025-03-31T19:37:57.691Z Learning: In the holidays library, when a locale file matches the country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language files should have filled msgstr fields with translations.Learnt from: KJhellico PR: vacanza/holidays#2394 File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88 Timestamp: 2025-03-31T19:37:57.691Z Learning: In the holidays library localization pattern, when a locale file matches a country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language locale files should have translations in the msgstr fields.holidays/locale/en_US/LC_MESSAGES/ST.po (2)
78-80: Properly translated observed holiday suffixThe observed holiday suffix format is correctly translated, which ensures observed holidays display properly in English.
30-76: English translations look goodAll holiday names are properly translated from Portuguese to English, maintaining accurate meanings and appropriate English terminology.
tests/countries/test_sao_tome_and_principe.py (2)
48-85: Tests for observed dates and special cases look goodYour tests for observed holidays, subdivisions, and the São Tomé Day introduction in 2019 are well-structured and verify important edge cases.
86-132: Localization tests are comprehensiveYour tests for default, Portuguese, and English localizations are thorough and verify all holiday translations correctly.
holidays/countries/sao_tome_and_principe.py (1)
107-113: Alias classes follow convention.The
STandSTPsubclasses are implemented correctly as simple pass‑through classes without docstrings, matching the repository’s alias pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
tests/countries/test_sao_tome_and_principe.py (1)
115-122: 🧹 Nitpick (assertive)Remove empty test methods
The test_special_holidays and test_variable_holidays methods don't test anything and could be removed to keep the code cleaner.
-def test_special_holidays(self): - # Example of testing special one-time holidays if any exist - pass - -def test_variable_holidays(self): - # Test holidays that change dates each year (like Carnival) - pass
♻️ Duplicate comments (1)
tests/countries/test_sao_tome_and_principe.py (1)
29-48: 🛠️ Refactor suggestionConsider restructuring tests by individual holidays instead of grouping
Based on project conventions, consider organizing tests by individual holidays rather than testing them in a group.
Here's how you could restructure this:
-def test_fixed_holidays(self): - fixed_dates = [ - ("01-01", "Ano Novo"), - ("02-03", "Dia dos Mártires"), - ("05-01", "Dia do Trabalhador"), - ("07-12", "Dia da Independência"), - ("09-06", "Dia das Forças Armadas"), - ("09-30", "Dia da Reforma Agrária"), - ("12-21", "Dia de São Tomé"), # Since 2019 - ("12-25", "Natal"), - ] - - for month_day, name in fixed_dates: - for year in range(2014, 2050): - # Skip São Tomé Day before 2019 - if month_day == "12-21" and year < 2019: - continue - - self.assertHolidayName(name, f"{year}-{month_day}") +def test_new_years_day(self): + self.assertHolidayName("Ano Novo", (f"{year}-01-01" for year in range(2014, 2050))) + +def test_martyrs_day(self): + self.assertHolidayName("Dia dos Mártires", (f"{year}-02-03" for year in range(2014, 2050))) + +def test_labor_day(self): + self.assertHolidayName("Dia do Trabalhador", (f"{year}-05-01" for year in range(2014, 2050))) + +def test_independence_day(self): + self.assertHolidayName("Dia da Independência", (f"{year}-07-12" for year in range(2014, 2050))) + +def test_armed_forces_day(self): + self.assertHolidayName("Dia das Forças Armadas", (f"{year}-09-06" for year in range(2014, 2050))) + +def test_agricultural_reform_day(self): + self.assertHolidayName("Dia da Reforma Agrária", (f"{year}-09-30" for year in range(2014, 2050))) + +def test_sao_tome_day(self): + # Only test from 2019 onwards + self.assertHolidayName("Dia de São Tomé", (f"{year}-12-21" for year in range(2019, 2050))) + # Verify it doesn't exist before 2019 + for year in range(2014, 2019): + self.assertNotIn(f"{year}-12-21", self.holidays) + +def test_christmas_day(self): + self.assertHolidayName("Natal", (f"{year}-12-25" for year in range(2014, 2050)))
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/countries/test_sao_tome_and_principe.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_sao_tome_and_principe.py (5)
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.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single asserti
3D11
on. Both approaches serve different testing purposes and complement each other.
🧬 Code Graph Analysis (1)
tests/countries/test_sao_tome_and_principe.py (2)
tests/common.py (3)
CommonCountryTests(356-374)assertHolidayName(195-199)assertLocalizedHolidays(327-338)holidays/countries/sao_tome_and_principe.py (3)
SaoTomeAndPrincipe(19-104)ST(107-108)STP(111-112)
🔇 Additional comments (6)
tests/countries/test_sao_tome_and_principe.py (6)
13-16: Good use of CommonCountryTestsYour imports correctly follow project conventions by using CommonCountryTests as the base class for test organization.
19-24: Correct test class setup structureThe test class definition and setup follow project conventions with proper inheritance and initialization. The year range covers the supported period from 2014 through 2050.
49-61: Good observed holidays testsThe tests correctly verify that holidays are observed on appropriate weekdays when they fall on weekends, and that observed rules only apply from 2020 onwards.
62-77: Well-structured subdivision testsYour tests for Príncipe subdivision holidays are thorough and correctly verify both subdivision-specific holidays and the presence of national holidays.
78-94: Complete default localization testsYour default localization tests thoroughly cover all national and subdivision holidays with their correct Portuguese names.
96-113: Complete English localization testsYour English localization tests provide good coverage for all holidays with their English translations.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2489 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 214 215 +1
Lines 13549 13588 +39
Branches 1930 1931 +1
=========================================
+ Hits 13549 13588 +39 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@tr33k, I recommend you to fix maximum suggestions before re-requesting PR review. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
holidays/locale/pt_ST/LC_MESSAGES/ST.po (1)
13-13: 🧹 Nitpick (assertive)Update file header description format
The file header description format should match the standard established in other localization files.
-# Sao Tome and Principe holidays. +# Sao Tome and Principe holidays. #This ensures consistency with other default language headers in the project.
holidays/locale/en_US/LC_MESSAGES/ST.po (2)
22-22:⚠️ Potential issueFix language setting in translation file header
The language field in the header is set to "pt" (Portuguese) although this is an en_US translation file. This could cause issues with locale selection.
-"Language: pt\n" +"Language: en_US\n"
28-28: 🛠️ Refactor suggestionFix source language setting in translation file header
The X-Source-Language field should be set to pt_ST (Portuguese for São Tomé and Príncipe) since that's the default language from which translations are made.
-"X-Source-Language: en_US\n" +"X-Source-Language: pt_ST\n"tests/countries/test_sao_tome_and_principe.py (1)
19-23: 🛠️ Refactor suggestionImprove test class setup by using the standard pattern
Consider switching to the standardized test setup used in other country implementations.
class TestSaoTomeAndPrincipe(CommonCountryTests, TestCase): - @classmethod - def setUpClass(cls): - super().setUpClass(SaoTomeAndPrincipe, years=range(2014, 2050)) - cls.observed_holidays = SaoTomeAndPrincipe(years=range(2014, 2050), observed=True) + def setUp(self): + super().setUp(SaoTomeAndPrincipe, years=range(2014, 2050)) + self.observed_holidays = SaoTomeAndPrincipe(years=range(2014, 2050), observed=True)This follows the convention used in other country test files and ensures proper test isolation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
holidays/countries/sao_tome_and_principe.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/ST.po(1 hunks)holidays/locale/pt_ST/LC_MESSAGES/ST.po(1 hunks)tests/countries/test_sao_tome_and_principe.py(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
holidays/locale/pt_ST/LC_MESSAGES/ST.po (1)
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. #"
holidays/locale/en_US/LC_MESSAGES/ST.po (1)
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. #"
tests/countries/test_sao_tome_and_principe.py (5)
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.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.
holidays/countries/sao_tome_and_principe.py (4)
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:86-88
Timestamp: 2025-04-23T09:22:41.742Z
Learning: For holiday definitions in the holidays package, keep comments simple with just the holiday name (e.g., "# Independence Day.") rather than including dates or historical context, as the function names already encode the date information.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
🔇 Additional comments (3)
holidays/locale/pt_ST/LC_MESSAGES/ST.po (2)
34-35:⚠️ Potential issueComplete translations for all holiday names
All the holiday translations have empty msgstr values. For the default language (pt_ST), these should be filled with the Portuguese names.
#. National holidays (observed everywhere) New Year's Day. msgid "Ano Novo" -msgstr "" +msgstr "Ano Novo"This pattern should be applied to all holiday translations in the file.
⛔ Skipped due to learnings
Learnt from: KJhellico PR: vacanza/holidays#2394 File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88 Timestamp: 2025-03-31T19:37:57.691Z Learning: In the holidays library, when a locale file matches the country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language files should have filled msgstr fields with translations.Learnt from: KJhellico PR: vacanza/holidays#2394 File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88 Timestamp: 2025-03-31T19:37:57.691Z Learning: In the holidays library localization pattern, when a locale file matches a country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language locale files should have translations in the msgstr fields.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.
30-31:⚠️ Potential issueFill in the missing translation for observed holidays
The format string for observed holidays has an empty msgstr value, which could lead to issues with displaying observed holiday names.
#, c-format msgid "%s (observado)" -msgstr "" +msgstr "%s (observado)"Since this is the default locale, the msgstr should contain the same value as msgid.
⛔ Skipped due to learnings
Learnt from: KJhellico PR: vacanza/holidays#2394 File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88 Timestamp: 2025-03-31T19:37:57.691Z Learning: In the holidays library, when a locale file matches the country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language files should have filled msgstr fields with translations.Learnt from: KJhellico PR: vacanza/holidays#2394 File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88 Timestamp: 2025-03-31T19:37:57.691Z Learning: In the holidays library localization pattern, when a locale file matches a country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language locale files should have translations in the msgstr fields.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.holidays/countries/sao_tome_and_principe.py (1)
58-61: Verify observed holiday start yearThe PR objectives mention observed holiday logic starting from 2014, but the code sets it to 2020.
kwargs.setdefault("observed_rule", SUN_TO_NEXT_MON + SAT_TO_PREV_FRI) # Holidays are observed on the next Monday if on Sunday, or previous Friday if on Saturday. # Based on common government practices since at least 2020. - kwargs.setdefault("observed_since", 2020) + kwargs.setdefault("observed_since", 2014)Ensure this aligns with the actual historical practice and update the comment accordingly.
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
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)
holidays/countries/sao_tome_and_principe.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/sao_tome_and_principe.py (6)
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:86-88
Timestamp: 2025-04-23T09:22:41.742Z
Learning: For holiday definitions in the holidays package, keep comments simple with just the holiday name (e.g., "# Independence Day.") rather than including dates or historical context, as the function names already encode the date information.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.486Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.486Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.
🔇 Additional comments (18)
holidays/countries/sao_tome_and_principe.py (18)
1-12: File header follows project conventions.
The header matches the standard format used across country modules, including copyright, authors, website, and license.
13-17: Imports look correct and necessary.
tr,ChristianHolidays,InternationalHolidays, andObservedHolidayBase(with its rules) are all required for this implementation.
19-27: Class signature and docstring adhere to naming conventions.
SaoTomeAndPrincipeinherits from the appropriate bases, and the ASCII docstring matches other country modules.
29-34: Attributes correctly configure country code and languages.
country,default_language,supported_languages, andobserved_labelalign with repository patterns.
35-43: Subdivision codes are comprehensive and sorted.
The tuple covers all six districts plus the autonomous region code.
44-54: Subdivision aliases mapping is accurate.
Names (with diacritics) map cleanly to their codes, reflecting local administrative labels.
56-61: Initialization sequence is correct.
Calling the Christian and International base constructors first, then settingobserved_ruleensures observed‐holiday logic is applied properly.
66-69: New Year's Day handling is implemented properly.
Both Jan 1 and the next‐year Jan 1 (for Saturday cases) use the correct observation rules.
71-73: King Amador Day is added correctly.
January 4 is set up with observed rules as expected.
74-76: Martyrs' Day setup is correct.
February 3 holiday and its observed behavior match project conventions.
77-79: Worker's Day implementation is consistent.
May 1 is localized and observed per the shared rules.
80-82: Independence Day is properly configured.
July 12 holiday and observed shift adhere to expectations.
83-85: Armed Forces Day is defined accurately.
September 6 mapping aligns with official holiday dates.
86-88: Agricultural Reform Day added.
September 30 holiday is implemented with correct observed logic.
89-91: São Tomé Day conditional inclusion is correct.
The holiday is only populated from 2019 onward, matching historical introduction.
93-95: Christmas Day is included with observed logic.
December 25 is correctly added via the ChristianHolidays base.
96-105: Subdivision-specific holidays for Príncipe are accurate.
Discovery of Príncipe Island, Autonomy Day, and São Lourenço Day all appear with proper dates and observations.
107-113: Alias classesSTandSTPare appropriately defined.
They subclassSaoTomeAndPrincipewithout extra docstrings, following project alias conventions.
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
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 (3)
holidays/countries/sao_tome_and_principe.py(1 hunks)holidays/locale/pt_ST/LC_MESSAGES/ST.po(1 hunks)tests/countries/test_sao_tome_and_principe.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
holidays/locale/pt_ST/LC_MESSAGES/ST.po (1)
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. #"
tests/countries/test_sao_tome_and_principe.py (5)
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.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.
holidays/countries/sao_tome_and_principe.py (6)
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:86-88
Timestamp: 2025-04-23T09:22:41.742Z
Learning: For holiday definitions in the holidays package, keep comments simple with just the holiday name (e.g., "# Independence Day.") rather than including dates or historical context, as the function names already encode the date information.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.486Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.486Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.
🧬 Code Graph Analysis (1)
holidays/countries/sao_tome_and_principe.py (3)
holidays/groups/christian.py (2)
ChristianHolidays(22-463)_add_christmas_day(208-216)holidays/groups/international.py (4)
InternationalHolidays(18-220)_add_new_years_day(126-134)_next_year_new_years_day(24-28)_add_labor_day(99-108)holidays/observed_holiday_base.py (1)
ObservedHolidayBase(101-243)
🔇 Additional comments (15)
holidays/locale/pt_ST/LC_MESSAGES/ST.po (2)
31-32: LGTM: Correct format string for observed holidays.The format string for observed holidays is correctly defined with the (observado) suffix, matching the implementation in the country class.
34-80: Holiday translations properly structured.The msgid entries for all holidays are correctly defined in Portuguese with empty msgstr fields, which is appropriate for the source language file. All the holidays match those defined in the implementation file.
tests/countries/test_sao_tome_and_principe.py (6)
13-17: LGTM: Correct imports and test class setup.The test class properly imports from unittest, the country module, and CommonCountryTests, following the project's standards for test organization.
19-27:Well-structured test class setup. The test class correctly extends CommonCountryTests and TestCase, and properly configures test years and observed years. The subdivision setup is also handled correctly.
29-34: LGTM: Good test coverage for aliases and boundaries.The tests for country aliases and the absence of holidays before the start year are correctly implemented, ensuring proper validation of the implementation boundaries.
35-97: Well-structured holiday tests.The tests are correctly organized by individual holidays rather than by years, following project conventions. Each test properly validates both regular and observed holiday dates.
99-127: Subdivision holiday tests properly implemented.The Príncipe-specific holiday tests correctly validate both the presence of holidays in the subdivision and their absence in the main country holidays.
129-164: LGTM: Comprehensive localization tests.The localization tests properly verify both default (Portuguese) and English (en_US) holiday names, ensuring the translations work correctly.
holidays/countries/sao_tome_and_principe.py (7)
19-27: LGTM: Class definition with proper inheritance and references.The class properly inherits from ObservedHolidayBase, ChristianHolidays, and InternationalHolidays. The references section includes relevant official sources.
29-54: LGTM: Well-defined class attributes.The country code, default language, supported languages, and subdivisions are all properly defined. The subdivision aliases are comprehensive and well-organized.
65-70: Excellent New Year's Day handling.The implementation correctly handles both regular New Year's Day and the special case for New Year's Day of the next year when December 31 falls on a Saturday.
71-94: LGTM: National holidays properly implemented.All national holidays are correctly implemented with appropriate translations and observed holiday support. The conditional for São Tomé Day (>= 2019) is correctly handled.
96-104: LGTM: Príncipe subdivision holidays properly implemented.The Príncipe-specific holidays are correctly implemented in the _populate_subdiv_p_public_holidays method, matching the test expectations.
107-112: LGTM: Simple country aliases.The country code aliases (ST and STP) are correctly implemented as pass-through classes without docstrings, following project conventions.
56-63:❌ Incorrect review comment
Observed holiday implementation might not match PR objectives.
The PR states that observed holiday logic starts from 2014, but the implementation sets it to 2020.
- kwargs.setdefault("observed_since", 2020) + kwargs.setdefault("observed_since", 2014)If observed rules should apply from the beginning of the holiday data, update this value to match the start_year.
I’ll inspect the
sao_tome_and_principe.pyfile to confirm the observed dates and start year.
🏁 Script executed:
#!/bin/bash set -e # Locate and inspect the Sao Tome & Principe holiday definition file=$(fd -t f sao_tome_and_principe.py) echo "File path: $file" echo "" echo "-- Lines 1–200 --" sed -n '1,200p' "$file" echo "" echo "-- observed_since usages --" grep -n "observed_since" "$file" echo "" echo "-- observed_rule usages --" grep -n "observed_rule" "$file" echo "" echo "-- class declaration --" grep -n "^class " "$file"Length of output: 651
I’ll re-run inspection focusing only on the main holiday definition file:
🏁 Script executed:
#!/bin/bash set -e # Locate only the main implementation file main_file=$(fd -t f sao_tome_and_principe.py | grep -v tests) echo "Inspecting: $main_file" echo "" echo "-- __init__ method (lines 1–200) --" sed -n '1,200p' "$main_file" echo "" echo "-- observed_since occurrences --" grep -n "observed_since" "$main_file"Length of output: 4469
No change needed for observed_since
The
observed_since = 2020line matches the comment right above it—government practice for São Tomé & Príncipe only began shifting weekend holidays in 2020. Keeping it at 2020 ensures observed rules aren’t applied before they were in effect.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
LGTM 👍
Great work @tr33k, thanks for taking care of this! And @KJhellico @PPsyrius for all the help!
|
Proposed change
Added holiday support for São Tomé and Príncipe (ST) comprising historical holiday definitions and observed holiday logic starting in 2014.
Included thorough tests covering all capability and edge situations as well.
I'm a novice and doing my best; if the contribution doesn't live up, feel free to assign it for more inspection or development.
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green