python-holidays icon indicating copy to clipboard operation
python-holidays copied to clipboard
verified publisher icon vacanza

Update Brazil holidays: Add São Paulo Capital subdivision support

Open avibrazil opened this issue 2 weeks ago • 6 comments

Proposed change

Adds São Paulo Capital subdivision which contains all SP subdivision holidays plus city anniversary.

Type of change

  • [X] New country/market holidays support (thank you!)
  • [ ] Supported country/market holidays update (calendar discrepancy fix, localization)
  • [ ] Existing code/documentation/test/process quality improvement (best practice, cleanup, refactoring, optimization)
  • [ ] Dependency update (version deprecation/pin/upgrade)
  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [ ] Breaking change (a code change causing existing functionality to break)
  • [ ] New feature (new holidays functionality in general)

Checklist

  • [X] I've read and followed the contributing guidelines.
  • [X] I've run make check locally; all checks and tests passed.

avibrazil avatar Dec 12 '25 15:12 avibrazil

Summary by CodeRabbit

  • New Features

    • Added São Paulo Capital as a distinct subdivision with city-level holidays; introduced São Paulo City Anniversary (Jan 25) from 1968.

  • Localization

    • Added translation entries for the city anniversary (en_US, uk) and added placeholder in pt_BR.

  • Documentation

    • README updated to note São Paulo Capital and highlight pt_BR.

  • Tests

    • Tests and snapshot updated to cover São Paulo Capital holidays.

  • Chores

    • CONTRIBUTORS updated.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Added a new Brazil subdivision "São Paulo Capital" with dedicated city-level holiday handling (delegates to SP state logic and adds Jan 25 city anniversary from 1968). Updated many per-subdivision holiday entries, translations, tests, a snapshot, README and CONTRIBUTORS.

Changes

Cohort / File(s) Summary
Brazil country logic
holidays/countries/brazil.py		 Added subdivision "São Paulo Capital" and alias; added _populate_subdiv_sao_paulo_capital_public_holidays() delegating to SP handlers and adding "Aniversário da Cidade de São Paulo" (year >= 1968). Updated public holiday routing and numerous per-state/subdivision holiday additions/adjustments (Black Awareness Day, Evangelical Day, state creation days, local entries).
Locale translations
holidays/locale/en_US/LC_MESSAGES/BR.po, holidays/locale/pt_BR/LC_MESSAGES/BR.po, holidays/locale/uk/LC_MESSAGES/BR.po		 Added msgid "Aniversário da Cidade de São Paulo" with translations (en_US, uk; pt_BR placeholder) and updated PO metadata (project/version, revision, translator, generator).
Tests
tests/countries/test_brazil.py		 Added tests for subdivision "São Paulo Capital" (years range), aligned Revolução Constitucionalista behavior with SP for that subdivision, added Jan 25 city-anniversary tests (1968+), and updated l10n expectations for en/pt/uk.
Snapshot data
snapshots/countries/BR_SÃO_PAULO_CAPITAL.json		 New snapshot mapping dates→holiday names for São Paulo Capital (1950–2050) including city anniversary and recurring/local holidays.
Docs & contributors
README.md, CONTRIBUTORS		 README: added "cities: São Paulo Capital" annotation to Brazil row and emphasized pt_BR; CONTRIBUTORS: appended Avi Alkalay.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • vacanza/holidays#2750 — Adds a city-level subdivision and a _populate_subdiv_<city>_public_holidays handler with tests and snapshot, following the same city-subdivision pattern.

Suggested reviewers

  • arkid15r
  • KJhellico

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding São Paulo Capital subdivision support to Brazil holidays, which matches the primary objective.
Description check ✅ Passed The description explains the change (adding São Paulo Capital subdivision with SP holidays plus city anniversary), references issue #3127, and follows contribution guidelines.
Linked Issues check ✅ Passed The changes implement all requirements from #3127: São Paulo Capital subdivision added, includes SP holidays plus January 25 city anniversary, follows naming convention guidance (full name vs SPC).
Out of Scope Changes check ✅ Passed All changes relate to the objective: subdivision definition, holiday logic, translations, tests, snapshot data, README documentation, and contributor credit are all in-scope for the feature.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 07a24d21291f166bf6f0c6dbeb600be1c45e9051 and 254c2b1168b32174a4a797eb5597ae0d86ad6866.

📒 Files selected for processing (1)
  • holidays/countries/brazil.py
🧰 Additional context used
🧠 Learnings (31)
📓 Common learnings 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py:209-209
Timestamp: 2025-09-14T17:17:14.387Z
Learning: In tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py, the explicit loop iteration pattern for subdivision-specific holiday checks (like Anniversary Day for TA subdivision) is intentionally preferred over using assertSubdivTa helper methods, as confirmed by PPsyrius.

Learnt from: KJhellico
Repo: vacanza/holidays PR: 2651
File: tests/countries/test_bonaire_sint_eustatius_and_saba.py:225-306
Timestamp: 2025-07-02T10:22:33.053Z
Learning: The `assertLocalizedHolidays` method in the vacanza/holidays project requires a complete list of all holidays from all subdivisions, not just subdivision-specific holidays. This is a framework requirement for proper localization testing.

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/france.py:178-182
Timestamp: 2025-06-18T10:18:59.447Z
Learning: In the France holidays implementation, deprecated subdivision names like "Alsace-Moselle" and "Saint-Barthélémy" are handled through explicit conditionals in _populate_public_holidays() that map them to their corresponding new subdivision-specific holiday population methods, providing backward compatibility while users transition to the new ISO codes.

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 3026
File: holidays/countries/spain.py:189-189
Timestamp: 2025-10-28T17:26:45.090Z
Learning: In the vacanza/holidays codebase, `_populate` methods (such as `_populate_public_holidays`, `_populate_subdiv_holidays`, and subdivision-specific methods like `_populate_subdiv_XX_public_holidays`) intentionally do not include return type annotations like `-> None`. This is a consistent pattern across country implementations (e.g., Germany, United States, Spain) and should not be changed even when static analysis tools like Ruff suggest adding them.
<!-- </add_learning>

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/france.py:184-206
Timestamp: 2025-06-18T10:58:28.058Z
Learning: In the holidays library France implementation, creating shared private methods between subdivision-specific holiday population methods (like _populate_subdiv_57_public_holidays and _populate_subdiv_6ae_public_holidays) is not supported by the current framework architecture, so duplicate code between functionally identical subdivision methods should be left as-is.

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/french_southern_territories.py:41-44
Timestamp: 2025-06-18T10:07:58.780Z
Learning: Territorial holiday classes that inherit from parent countries (like HolidaysAX from Finland, HolidaysSJ from Norway, HolidaysTF from France) follow a standard pattern of silently overriding self.subdiv in their _populate_public_holidays() method without validation, as this ensures they always use the correct subdivision code for their territory regardless of user input.

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2407
File: snapshots/countries/TL_COMMON.json:7-7
Timestamp: 2025-04-03T05:59:57.480Z
Learning: In the holidays project, snapshot files (like snapshots/countries/TL_COMMON.json) are auto-generated when running `make snapshot` and should not be manually edited. Semicolons (;) in holiday entries are used as separators when multiple holidays occur on the same date.

Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/france.py:178-180
Timestamp: 2025-06-18T10:26:50.180Z
Learning: In the France holidays implementation, the _populate_subdiv_57_public_holidays() (Moselle) and _populate_subdiv_6ae_public_holidays() (Alsace) methods are functionally identical, both adding Good Friday (≥1893) and Saint Stephen's Day (≥1892) with the same legal references from August 16th, 1892. Therefore, for the deprecated "Alsace-Moselle" subdivision, calling either method produces the same result.
📚 Learning: 2025-10-28T17:26:45.090Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 3026
File: holidays/countries/spain.py:189-189
Timestamp: 2025-10-28T17:26:45.090Z
Learning: In the vacanza/holidays codebase, `_populate` methods (such as `_populate_public_holidays`, `_populate_subdiv_holidays`, and subdivision-specific methods like `_populate_subdiv_XX_public_holidays`) intentionally do not include return type annotations like `-> None`. This is a consistent pattern across country implementations (e.g., Germany, United States, Spain) and should not be changed even when static analysis tools like Ruff suggest adding them.
<!-- </add_learning>

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-06-18T10:58:28.058Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/france.py:184-206
Timestamp: 2025-06-18T10:58:28.058Z
Learning: In the holidays library France implementation, creating shared private methods between subdivision-specific holiday population methods (like _populate_subdiv_57_public_holidays and _populate_subdiv_6ae_public_holidays) is not supported by the current framework architecture, so duplicate code between functionally identical subdivision methods should be left as-is.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-06-18T10:07:58.780Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/french_southern_territories.py:41-44
Timestamp: 2025-06-18T10:07:58.780Z
Learning: Territorial holiday classes that inherit from parent countries (like HolidaysAX from Finland, HolidaysSJ from Norway, HolidaysTF from France) follow a standard pattern of silently overriding self.subdiv in their _populate_public_holidays() method without validation, as this ensures they always use the correct subdivision code for their territory regardless of user input.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-06-18T10:18:59.447Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/france.py:178-182
Timestamp: 2025-06-18T10:18:59.447Z
Learning: In the France holidays implementation, deprecated subdivision names like "Alsace-Moselle" and "Saint-Barthélémy" are handled through explicit conditionals in _populate_public_holidays() that map them to their corresponding new subdivision-specific holiday population methods, providing backward compatibility while users transition to the new ISO codes.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-06-18T10:26:50.180Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/france.py:178-180
Timestamp: 2025-06-18T10:26:50.180Z
Learning: In the France holidays implementation, the _populate_subdiv_57_public_holidays() (Moselle) and _populate_subdiv_6ae_public_holidays() (Alsace) methods are functionally identical, both adding Good Friday (≥1893) and Saint Stephen's Day (≥1892) with the same legal references from August 16th, 1892. Therefore, for the deprecated "Alsace-Moselle" subdivision, calling either method produces the same result.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-03-30T20:18:46.006Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2386
File: holidays/countries/nepal.py:24-26
Timestamp: 2025-03-30T20:18:46.006Z
Learning: In the holidays library, country classes do not directly implement `_populate()`. Instead, they implement specialized methods like `_populate_public_holidays()`, and the base class `HolidayBase` handles the orchestration by calling these specialized methods.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-09-14T17:17:14.387Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py:209-209
Timestamp: 2025-09-14T17:17:14.387Z
Learning: In tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py, the explicit loop iteration pattern for subdivision-specific holiday checks (like Anniversary Day for TA subdivision) is intentionally preferred over using assertSubdivTa helper methods, as confirmed by PPsyrius.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-07-02T10:22:33.053Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2651
File: tests/countries/test_bonaire_sint_eustatius_and_saba.py:225-306
Timestamp: 2025-07-02T10:22:33.053Z
Learning: The `assertLocalizedHolidays` method in the vacanza/holidays project requires a complete list of all holidays from all subdivisions, not just subdivision-specific holidays. This is a framework requirement for proper localization testing.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-06-16T12:28:31.641Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2632
File: holidays/countries/solomon_islands.py:95-98
Timestamp: 2025-06-16T12:28:31.641Z
Learning: Library-wide holiday patterns and their optimizations should be handled at the base class level (like InternationalHolidays) rather than documenting workarounds in individual country modules. This maintains separation of concerns and avoids documentation duplication.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-06-16T11:46:35.303Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2638
File: holidays/countries/svalbard_and_jan_mayen.py:26-32
Timestamp: 2025-06-16T11:46:35.303Z
Learning: When creating country holiday aliases that inherit from parent countries (like Svalbard and Jan Mayen from Norway, or Åland from Finland), it's standard practice to hardcode a specific subdivision code in the _populate methods since the subdivision codes for the parent country yield identical results. The Åland Islands implementation serves as the reference pattern for this approach.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-10-28T17:02:23.997Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 3025
File: holidays/exceptions.py:14-25
Timestamp: 2025-10-28T17:02:23.997Z
Learning: Do not suggest adding return type annotations (such as `-> None` for `__init__` methods) in the vacanza/holidays codebase.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-04-23T09:59:19.886Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2490
File: holidays/countries/ethiopia.py:45-45
Timestamp: 2025-04-23T09:59:19.886Z
Learning: For the Ethiopia holidays class, it's appropriate to add a return type hint only to the `_is_leap_year` method to match the base class implementation in `holidays/holiday_base.py`, while keeping other methods without type hints to maintain consistency with other country implementations.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-11-08T04:57:36.307Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_cambodia.py:20-22
Timestamp: 2025-11-08T04:57:36.307Z
Learning: In the vacanza/holidays test suite, `setUpClass` methods in test files intentionally do not include return type annotations like `-> None`. This is a consistent pattern across all country test files and should not be changed, even when static analysis tools like Ruff suggest adding them.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-11-08T04:57:50.267Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_cameroon.py:21-23
Timestamp: 2025-11-08T04:57:50.267Z
Learning: In the vacanza/holidays test suite, do not suggest adding return type annotations (such as `-> None`) to `setUpClass` class methods in test files. This is a consistent pattern across the test suite and should be maintained even when static analysis tools like Ruff suggest adding them.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-04-02T18:38:35.164Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2398
File: tests/countries/test_guinea.py:237-239
Timestamp: 2025-04-02T18:38:35.164Z
Learning: In the vacanza/holidays project, the method assertLocalizedHolidays in country test files should be called with positional parameters rather than named parameters to maintain consistency with the rest of the codebase.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-04-25T20:27:59.086Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2402
File: holidays/countries/trinidad_and_tobago.py:85-92
Timestamp: 2025-04-25T20:27:59.086Z
Learning: The `_populate_observed` method in holiday classes should maintain the same signature as the parent class `ObservedHolidayBase`, even if specific child class implementations don't use all parameters.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-04-03T16:58:27.175Z 
Learnt from: Wasif-Shahzad
Repo: vacanza/holidays PR: 2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants internally through parent class implementations even when these constants don't appear directly in the file. Removing imports that seem unused based on a simple text search could break functionality.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-09-17T15:53:16.940Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2944
File: holidays/countries/myanmar.py:108-111
Timestamp: 2025-09-17T15:53:16.940Z
Learning: In the holidays package, explanatory comments about year restrictions or policy context should be placed above conditional blocks or at method level, never directly above holiday name comments that precede tr() function calls, as this would include the explanatory text in .po localization files when running `make l10n`.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-08-21T04:56:03.780Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
Learning: In the holidays library, countries with localization support consistently use tr() wrappers around holiday names when calling _add_* methods (e.g., self._add_new_years_day(tr("Holiday Name"))). This is the established pattern across United States, Thailand, and other l10n-enabled countries, contrary to any suggestion that translation is handled internally by _add_* methods.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-10-27T21:23:12.690Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 3008
File: holidays/financial/_sifma.py:146-150
Timestamp: 2025-10-27T21:23:12.690Z
Learning: In the holidays library, `_christmas_day` is a property defined in the `ChristianHolidays` mixin class and is always available to any class that inherits from `ChristianHolidays`, regardless of whether `_populate_public_holidays` is called or which categories are requested. It does not depend on populating public holidays.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-09-18T07:01:12.236Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2942
File: holidays/countries/south_africa.py:91-98
Timestamp: 2025-09-18T07:01:12.236Z
Learning: In the holidays library South Africa implementation, inline ternary operators for holiday name selection (like "Republic Day" if self._year >= 1961 else "Union Day") are intentionally kept inline rather than extracted to separate variables, as this structure is designed in preparation for future localization (l10n) support where the contextual relationship between conditions and translatable strings needs to be preserved.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-08-21T04:56:03.780Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
Learning: In the holidays library, countries with localization support DO use tr() wrappers around holiday names when calling _add_* methods. This is the correct pattern for l10n-enabled country implementations, contrary to previous learning about translation being handled internally by _add_* methods.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-06-14T11:04:31.180Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2609
File: holidays/countries/nauru.py:57-60
Timestamp: 2025-06-14T11:04:31.180Z
Learning: In the holidays library, the base `HolidayBase._populate()` method already includes a guard clause that prevents holiday population methods like `_populate_public_holidays()` from being called when the year is before `start_year` or after `end_year`. Therefore, individual country implementations do not need to add their own guard clauses for years before independence or other start dates.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-08-19T19:47:21.735Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2829
File: tests/countries/test_canada.py:291-296
Timestamp: 2025-08-19T19:47:21.735Z
Learning: In the holidays library, HolidayBase objects automatically populate years on-demand when expand=True (the default). When checking dates from years not initially in the years range, those years are automatically populated via the logic at line 646 in holiday_base.py: "if self.expand and dt.year not in self.years:". This means tests can check dates outside the initial year range without needing separate holiday instances.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-07-14T20:23:48.198Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2654
File: holidays/countries/cabo_verde.py:133-141
Timestamp: 2025-07-14T20:23:48.198Z
Learning: The holidays library provides helper methods `_add_holiday_2nd_sun_of_may()` and `_add_holiday_3rd_sun_of_jun()` for adding holidays on the 2nd Sunday of May and 3rd Sunday of June respectively. These methods are used across multiple country implementations including Latvia, Finland, Belarus, Malaysia, Madagascar, and Cape Verde.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-09-14T04:41:10.139Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_south_africa.py:22-22
Timestamp: 2025-09-14T04:41:10.139Z
Learning: South Africa's observed holiday system only started in 1995, so in tests/countries/test_south_africa.py, using years_non_observed=range(1995, 2050) is intentional to limit testing to years where observed holidays actually exist, improving both correctness and performance.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-07-10T03:36:16.461Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2706
File: holidays/countries/cayman_islands.py:80-97
Timestamp: 2025-07-10T03:36:16.461Z
Learning: In the holidays library, date dictionaries that map years to specific dates (like queens_birthday_dates, special holiday dates, etc.) are typically defined within the _populate_public_holidays method rather than as module-level constants. This is the established library-wide pattern and should be maintained for consistency.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-06-16T14:08:09.492Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2635
File: holidays/countries/bhutan.py:13-16
Timestamp: 2025-06-16T14:08:09.492Z
Learning: In the holidays library, translation is performed in the `_add_holiday` methods, not at the string level. Holiday strings passed to `_add_holiday_*` methods are processed through the translation machinery within those methods, so using `tr()` or `self.tr()` wrappers is unnecessary.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-05-10T04:02:13.815Z 
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2537
File: holidays/countries/finland.py:249-253
Timestamp: 2025-05-10T04:02:13.815Z
Learning: Holiday name comments directly above tr() function calls in the holidays package should only contain the holiday name itself (e.g., "# Independence Day.") without any additional context, dates, or historical information.

Applied to files:

  • holidays/countries/brazil.py
📚 Learning: 2025-12-15T22:50:33.654Z 
Learnt from: KJhellico
Repo: vacanza/holidays PR: 3135
File: holidays/countries/united_states.py:1175-1178
Timestamp: 2025-12-15T22:50:33.654Z
Learning: In the holidays library (vacanza/holidays), for Puerto Rico and potentially other US territories, maintain the documented separation: place Spanish holiday name comments on one line, followed by a blank line, and then the English localization comment. Do not remove the blank line separator, as it documents the Spanish name (e.g., 'Día de la Abolición de la Esclavitud') while the English name below (e.g., 'Emancipation Day') is the actual localization.

Applied to files:

  • holidays/countries/brazil.py
🧬 Code graph analysis (1)
holidays/countries/brazil.py (1)
holidays/countries/italy.py (1)
  • _populate_subdiv_sp_public_holidays (847-849)
🪛 Ruff (0.14.10)
holidays/countries/brazil.py

238-238: Missing return type annotation for private function _populate_subdiv_am_public_holidays

Add return type annotation: None

(ANN202)

359-359: Missing return type annotation for private function _populate_subdiv_rr_public_holidays

Add return type annotation: None

(ANN202)

392-392: Missing return type annotation for private function _populate_subdiv_sao_paulo_capital_public_holidays

Add return type annotation: None

(ANN202)

🔇 Additional comments (5)
holidays/countries/brazil.py (5)

31-32: Reference link properly placed.

The São Paulo Capital reference is correctly added to the References section with appropriate web archive link formatting.

67-68: Subdivision addition follows repository conventions.

The city-level subdivision is properly added with a comment separator, following the pattern used for other geographic subdivisions in the repository.

188-189: Condition correctly enables pre-1996 subdivision holidays for São Paulo Capital.

The OR condition properly allows São Paulo Capital to populate subdivision holidays for years before 1996 (when the city anniversary started in 1968), while maintaining the 1996 requirement for other subdivisions. The municipal law reference is appropriately documented.

392-398: Implementation correctly delegates to SP and adds city anniversary.

The method properly:

  • Delegates to _populate_subdiv_sp_public_holidays() to inherit all São Paulo state holidays
  • Adds the city anniversary for years >= 1968 (matching the 1967 law effective date)
  • Uses tr() wrapper for localization support
  • Documents the municipal law reference

The static analysis warning about missing return type annotation should be ignored—per codebase conventions, _populate methods intentionally omit return type annotations across all country implementations.

Based on learnings, _populate methods (including subdivision-specific methods) in the vacanza/holidays codebase intentionally do not include return type annotations to maintain consistency with established patterns.

162-163: This explicit call is necessary and doesn't cause duplicates. The framework normalizes "São Paulo Capital" to "São_Paulo_Capital" (preserving non-ASCII characters), then searches for _populate_subdiv_São_Paulo_Capital_public_holidays. Since the method is named _populate_subdiv_sao_paulo_capital_public_holidays (lowercase ASCII), the framework cannot auto-discover it, so the explicit call on line 163 is the only way these holidays get populated.

Likely an incorrect or invalid review comment.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 12 '25 15:12 coderabbitai[bot]

Does "SPC" code value have any official origin?

KJhellico avatar Dec 12 '25 16:12 KJhellico

Does "SPC" code value have any official origin?

No. It’s just the obvious choice.

avibrazil avatar Dec 12 '25 21:12 avibrazil

No. It’s just the obvious choice.

In that case, it might be better to refer to it as "São Paulo Capital" in full directly without creating unofficial ISO codes, see Germany's Augsburg, New Zealand's South Canterbury, and Italy's city-level holidays. 👍

PPsyrius avatar Dec 12 '25 23:12 PPsyrius

see Germany's Augsburg, New Zealand's South Canterbury, and Italy's city-level holidays. 👍

I was not even aware of those existing ones. Let me see them and I'll send a new pull request.

avibrazil avatar Dec 13 '25 09:12 avibrazil

In that case, it might be better to refer to it as "São Paulo Capital" in full directly without creating unofficial ISO codes, see Germany's Augsburg, New Zealand's South Canterbury, and Italy's city-level holidays. 👍

OK, updated the branch code following these tips. Now it is based on New Zealand's South Canterbury

Please accept PR

Thank you in advance

avibrazil avatar Dec 15 '25 19:12 avibrazil

BTW I can't test the code. Build system make check fails for me in command uv run --no-sync pre-commit run --all-files with error

error: Failed to spawn: `pre-commit`
  Caused by: No such file or directory (os error 2)

avibrazil avatar Dec 15 '25 19:12 avibrazil 

error: Failed to spawn: `pre-commit`
  Caused by: No such file or directory (os error 2)

Do you have uv installed? We use it since #3101.

KJhellico avatar Dec 15 '25 19:12 KJhellico

Do you have uv installed? We use it since #3101.

Yes, I have. Other previous commands on that Makefile did work. I don't know why that one failed.

avibrazil avatar Dec 15 '25 23:12 avibrazil

Try uv sync --all-groups or re-initialize environment completely with make setup

KJhellico avatar Dec 16 '25 00:12 KJhellico

Yes, I have. Other previous commands on that Makefile did work.

Check out WSL-specific section for more details if you're a Windows-based contributor https://holidays.readthedocs.io/en/latest/contributing/ - this works for me personally

PPsyrius avatar Dec 16 '25 03:12 PPsyrius

Folks I'm spending hours already trying to update this. make check fails but doesn't say why.

The build system is way more complex than the module itself. I just want to add a subdivision called "São Paulo Capital" which is the same as Brazil::SP plus the city anniversary on January 25, starting on 1967 according to this decree.

Can anyone please finish this for me? The feature/spc-br-subdivision branch in my fork already has all the relevant changes.

Thank you in advance

avibrazil avatar Dec 18 '25 19:12 avibrazil

BTW, I can't push the latest changes to my branch on my fork because make check doesn't pass.

avibrazil avatar Dec 18 '25 19:12 avibrazil 

git add .
git commit --no-verify
git push --no-verify

This should work to help bypass the local pre-commit checks IMO, and I'll take it from there if needed

PPsyrius avatar Dec 18 '25 19:12 PPsyrius

Thanks a lot…. I didn't know about —no-verify Latest updates pushed to https://github.com/avibrazil/holidays/tree/feature/spc-br-subdivision I suspect of circular dependency between SP and SPC, but can't test it

avibrazil avatar Dec 18 '25 19:12 avibrazil

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 100.00%. Comparing base (8323b18) to head (254c2b1).

Additional details and impacted files 
@@            Coverage Diff            @@
##               dev     #3128   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          306       306           
  Lines        18249     18255    +6     
  Branches      2327      2329    +2     
=========================================
+ Hits         18249     18255    +6

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Dec 24 '25 04:12 codecov[bot]

AFAIK we forget to override years_subdiv_são_paulo_capital so that's why the coverage tests keep failing, I've also converted _populate_subdiv_spc_public_holidays to _populate_subdiv_são_paulo_capital_public_holidays directly as well.

Should be more or less ready for PR review again 🎉

PPsyrius avatar Dec 24 '25 04:12 PPsyrius

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sonarqubecloud[bot] avatar Dec 25 '25 05:12 sonarqubecloud[bot]