Skip to content

Conversation

Wasif-Shahzad
Copy link
Contributor

@Wasif-Shahzad Wasif-Shahzad commented Jun 4, 2025

Proposed change

Add Senegal holidays.
Resolves #1266

Type of change

  • 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

Copy link
Contributor

coderabbitai bot commented Jun 4, 2025

Summary by CodeRabbit

  • New Features

    • Added support for Senegal's public holidays, including both fixed and Islamic holidays, with coverage from 1964 onward.
    • Introduced localization for Senegal holidays in both French (fr_SN) and English (en_US).
    • Senegal holidays now support observed rules and estimated date labels for Islamic holidays.
  • Documentation

    • Updated documentation to include Senegal as a supported country.
  • Tests

    • Added comprehensive tests for Senegal's holiday calendar, aliases, observed rules, and localization.

Summary by CodeRabbit

  • New Features

    • Added support for Senegal public holidays, including both fixed Gregorian and Islamic holidays.
    • Introduced localization for Senegal holiday names in French (fr_SN) and English (en_US).
    • Added the Grand Magal of Touba as a recognized Islamic holiday.
  • Documentation

    • Updated documentation to reflect Senegal as a supported country.
  • Tests

    • Added comprehensive tests for Senegal's holiday calendar and localization.

Walkthrough

Senegal holiday support was added, including a new country module, localization files, and test coverage. The Islamic calendar was expanded to support the Grand Magal of Touba. Documentation and registries were updated to reflect the new country, and relevant methods/constants were introduced for holiday calculation and localization.

Changes

Files/Paths Change Summary
holidays/countries/senegal.py, holidays/locale/en_US/LC_MESSAGES/SN.po, holidays/locale/fr_SN/LC_MESSAGES/SN.po Added Senegal holiday module and localization files for French and English.
holidays/calendars/islamic.py, holidays/groups/islamic.py, scripts/calendar/islamic_generator.py Added Grand Magal of Touba holiday constant, date mapping, and support in Islamic calendar logic.
holidays/countries/init.py, holidays/registry.py Registered Senegal in country imports and registry.
README.md Updated documentation to list Senegal as a supported country.
tests/countries/test_senegal.py Added comprehensive tests for Senegal holiday logic and localization.

Assessment against linked issues

Objective Addressed Explanation
Add Senegal holidays (fixed, Christian, Islamic, localization) (#1266)
Register Senegal in country list and documentation (#1266)
Provide test coverage for Senegal holidays (#1266)
Add Grand Magal of Touba to Islamic calendar (#1266)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes detected.

Suggested reviewers

  • PPsyrius
  • KJhellico
✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Jun 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (8b4f02b) to head (fbe5816).
Report is 7 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff            @@
##               dev     #2593   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          229       230    +1     
  Lines        14571     14636   +65     
  Branches      2039      2045    +6     
=========================================
+ Hits         14571     14636   +65     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d2fde70 and 758c02a.

📒 Files selected for processing (10)
  • README.md (2 hunks)
  • holidays/calendars/islamic.py (3 hunks)
  • holidays/countries/__init__.py (1 hunks)
  • holidays/countries/senegal.py (1 hunks)
  • holidays/groups/islamic.py (1 hunks)
  • holidays/locale/en_US/LC_MESSAGES/SN.po (1 hunks)
  • holidays/locale/fr/LC_MESSAGES/SN.po (1 hunks)
  • holidays/registry.py (1 hunks)
  • scripts/calendar/islamic_generator.py (1 hunks)
  • tests/countries/test_senegal.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
holidays/countries/__init__.py (1)
holidays/countries/senegal.py (3)
  • Senegal (33-103)
  • SN (106-107)
  • SEN (110-111)
holidays/groups/islamic.py (1)
holidays/calendars/islamic.py (1)
  • grand_magal_of_touba_dates (3993-3994)
tests/countries/test_senegal.py (2)
tests/common.py (6)
  • TestCase (28-338)
  • CommonCountryTests (356-374)
  • assertAliases (121-130)
  • assertNoHolidays (292-294)
  • assertHolidayName (195-199)
  • assertLocalizedHolidays (327-338)
holidays/countries/senegal.py (3)
  • Senegal (33-103)
  • SN (106-107)
  • SEN (110-111)
holidays/calendars/islamic.py (3)
holidays/calendars/hindu.py (1)
  • _get_holiday (1398-1402)
holidays/calendars/buddhist.py (1)
  • _get_holiday (430-434)
holidays/calendars/chinese.py (1)
  • _get_holiday (1325-1332)
🪛 Pylint (3.3.7)
holidays/groups/islamic.py

[error] 232-232: Instance of 'IslamicHolidays' has no '_year' member

(E1101)

tests/countries/test_senegal.py

[convention] 1-1: Missing module docstring

(C0114)


[convention] 19-19: Missing class docstring

(C0115)


[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSenegal.setUpClass' method

(W0221)


[convention] 26-26: Missing function or method docstring

(C0116)


[convention] 29-29: Missing function or method docstring

(C0116)


[convention] 32-32: Missing function or method docstring

(C0116)


[convention] 36-36: Missing function or method docstring

(C0116)


[convention] 40-40: Missing function or method docstring

(C0116)


[convention] 44-44: Missing function or method docstring

(C0116)


[convention] 48-48: Missing function or method docstring

(C0116)


[convention] 52-52: Missing function or method docstring

(C0116)


[convention] 56-56: Missing function or method docstring

(C0116)


[convention] 67-67: Missing function or method docstring

(C0116)


[convention] 78-78: Missing function or method docstring

(C0116)


[convention] 89-89: Missing function or method docstring

(C0116)


[convention] 101-101: Missing function or method docstring

(C0116)


[convention] 113-113: Missing function or method docstring

(C0116)


[convention] 125-125: Missing function or method docstring

(C0116)


[convention] 138-138: Missing function or method docstring

(C0116)


[convention] 150-150: Missing function or method docstring

(C0116)


[convention] 168-168: Missing function or method docstring

(C0116)

holidays/calendars/islamic.py

[convention] 3993-3993: Missing function or method docstring

(C0116)

holidays/countries/senegal.py

[convention] 1-1: Missing module docstring

(C0114)


[warning] 48-48: Keyword argument before variable positional arguments list in the definition of init function

(W1113)


[convention] 106-106: Missing class docstring

(C0115)


[convention] 110-110: Missing class docstring

(C0115)


[convention] 114-114: Missing class docstring

(C0115)

🔇 Additional comments (17)
scripts/calendar/islamic_generator.py (1)

41-42: LGTM - Clean addition of Senegal's Islamic holiday.

The Grand Magal of Touba entry follows the established pattern perfectly and is correctly positioned with proper documentation.

holidays/registry.py (1)

168-168: Perfect registry entry for Senegal.

Correctly formatted with proper ISO codes and alphabetical positioning.

holidays/countries/__init__.py (1)

160-160: Import statement looks good.

Follows the established pattern and is properly positioned alphabetically.

README.md (2)

108-108: Country count correctly updated.

Proper increment from 183 to 184 countries with the addition of Senegal.


1165-1170: Well-formatted documentation entry for Senegal.

Table entry includes all required information with proper HTML formatting and language specifications.

holidays/locale/en_US/LC_MESSAGES/SN.po (1)

1-90: Solid localization implementation.

The English translation file is well-structured and follows proper gettext conventions. The translations accurately capture the holiday meanings and the estimated format string is correctly implemented.

holidays/locale/fr/LC_MESSAGES/SN.po (1)

1-89: Correct French localization setup.

Empty msgstr fields are appropriate here since French is the source language (as indicated by X-Source-Language: fr). The structure matches the English file perfectly.

holidays/calendars/islamic.py (2)

29-29: Good constant addition.

The new Islamic holiday constant follows the established naming pattern in the file.


1455-1610: Comprehensive date mapping.

The date dictionary covers an extensive range (1924-2077) and follows the same structure as other Islamic holidays. The tuple format for multiple dates per year is handled correctly.

tests/countries/test_senegal.py (5)

19-24: Solid test setup with comprehensive coverage.

The test class properly inherits from CommonCountryTests and sets up both regular and non-estimated Islamic holiday instances. The year range 1960-2050 correctly aligns with Senegal's independence.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 19-19: Missing class docstring

(C0115)


[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSenegal.setUpClass' method

(W0221)


32-54: Fixed holidays correctly tested across full date range.

All fixed holidays (New Year's, Independence Day, Labor Day, Assumption, All Saints', Christmas) are properly tested with the correct dates and French names.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 32-32: Missing function or method docstring

(C0116)


[convention] 36-36: Missing function or method docstring

(C0116)


[convention] 40-40: Missing function or method docstring

(C0116)


[convention] 44-44: Missing function or method docstring

(C0116)


[convention] 48-48: Missing function or method docstring

(C0116)


[convention] 52-52: Missing function or method docstring

(C0116)


89-148: Islamic holidays comprehensively tested.

Good coverage of all Islamic holidays with both specific date verification and full range testing. The dual testing approach with self.no_estimated_holidays ensures the estimated date handling works correctly.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 89-89: Missing function or method docstring

(C0116)


[convention] 101-101: Missing function or method docstring

(C0116)


[convention] 113-113: Missing function or method docstring

(C0116)


[convention] 125-125: Missing function or method docstring

(C0116)


[convention] 138-138: Missing function or method docstring

(C0116)


150-185: Localization testing covers both supported languages.

The localization tests properly verify holiday names in both French (default) and English (en_US), covering all holidays for 2024. Nice comprehensive verification.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 150-150: Missing function or method docstring

(C0116)


[convention] 168-168: Missing function or method docstring

(C0116)


56-87:

✅ Verification successful

Christian movable holidays properly verified.

The test dates for Easter Monday, Ascension Day, and Whit Monday look accurate for the sample years tested.

Let me verify a couple of these Easter-related dates:


🏁 Script executed:

#!/bin/bash
# Verify Easter Monday dates for accuracy
# Easter Monday is the day after Easter Sunday

python3 -c "
import datetime
from dateutil.easter import easter

# Check a few years
years = [2020, 2022, 2024]
for year in years:
    easter_sunday = easter(year)
    easter_monday = easter_sunday + datetime.timedelta(days=1)
    print(f'{year}: Easter Monday = {easter_monday.strftime(\"%Y-%m-%d\")}')
"

Length of output: 410


Christian movable holiday tests confirmed

Verified Easter Monday for 2020, 2022, and 2024 against dateutil.easter—all match the test data. The Ascension Day and Whit Monday samples also look accurate. No further changes needed.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 56-56: Missing function or method docstring

(C0116)


[convention] 67-67: Missing function or method docstring

(C0116)


[convention] 78-78: Missing function or method docstring

(C0116)

holidays/countries/senegal.py (3)

62-103: Holiday definitions look comprehensive and correct.

Good coverage of fixed holidays, Christian movable holidays, and Islamic holidays. The French names are properly localized using gettext.


106-111: Simple alias classes are appropriate.

The SN and SEN alias classes follow the standard pattern used throughout the codebase.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 106-106: Missing class docstring

(C0115)


[convention] 110-110: Missing class docstring

(C0115)


48-60: 🧹 Nitpick (assertive)

Improve parameter order in constructor.

Move the keyword argument after *args to follow Python conventions.

-    def __init__(self, islamic_show_estimated: bool = True, *args, **kwargs):
+    def __init__(self, *args, islamic_show_estimated: bool = True, **kwargs):
⛔ Skipped due to learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2583
File: holidays/countries/niger.py:71-71
Timestamp: 2025-05-31T03:21:53.248Z
Learning: In the holidays Python library, the standard pattern for country class __init__ methods is to have keyword arguments with defaults before *args and **kwargs, such as `def __init__(self, islamic_show_estimated: bool = True, *args, **kwargs):`. This pattern is used consistently across all country implementations and should not be flagged as an issue.
🧰 Tools
🪛 Pylint (3.3.7)

[warning] 48-48: Keyword argument before variable positional arguments list in the definition of init function

(W1113)

Copy link
Collaborator

@PPsyrius PPsyrius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start ❤️

@KJhellico
Copy link
Collaborator

Law 74-52
Law 83-54

@KJhellico KJhellico changed the title Add Senegal holidays. Add Senegal holidays Jun 4, 2025
@Wasif-Shahzad
Copy link
Contributor Author

There's one more confusing thing. In the law Law 74-52, it says that Eid al-Fitr and Eid al-Adha would observed on a Sunday-to-Monday basis if it falls on Sunday and no other law discusses this. What to do?

Translation done by ChatGPT for this

@PPsyrius
Copy link
Collaborator

PPsyrius commented Jun 5, 2025

In the law Law 74-52, it says that Eid al-Fitr and Eid al-Adha would observed on a Sunday-to-Monday basis if it falls on Sunday and no other law discusses this.

AFAIK Law 83-54 didn't touch it either aside from adding Ashura and Senegambia Confederation Day, so I assume it exists at least until the subsequent revision in 1989 it supposedly still exists and are de facto grant "Bridge Holiday" (Jour de Pont) according to this FB post from Apr 20, 2025 i.e. Decree 2018-1942

Another interesting thing is that according to Article 4, Independence Day and Labor Day would've been classified as WORKDAY category ( Art. 4.- La fête nationale et la journée du premier mai sont chômées et payées,) and wasn't removed until 2014 in Law 2013-16.

@PPsyrius
Copy link
Collaborator

PPsyrius commented Jun 5, 2025

There's also a Special Holiday for DEC 26, 2022 as declared in Conseil des ministres du 22 décembre 2022

@Wasif-Shahzad
Copy link
Contributor Author

it supposedly still exists and are de facto grant "Bridge Holiday" (Jour de Pont)

What do you mean by this? @PPsyrius
timeanddate.com also has observances for Eid al-Fitr and Eid al-Adha btw

@PPsyrius
Copy link
Collaborator

PPsyrius commented Jun 5, 2025

What I meant is that the SUN_TO_NEXT_MON observances as stated in Law 74-52 are still in place for both Eid al-Fitr and Eid al-Adha in Senegal.

For others, i.e. Decree 2018-1942 and Conseil des ministres du 22 décembre 2022 - let's put those in as special holidays for now.

@KJhellico
Copy link
Collaborator

Another interesting thing is that according to Article 4, Independence Day and Labor Day would've been classified as WORKDAY category ( Art. 4.- La fête nationale et la journée du premier mai sont chômées et payées,)

I understand this phrase as "exactly these holidays are non-working and paid (and the rest of holidays are non-working and unpaid).

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5b5fa0 and 20c3db2.

📒 Files selected for processing (4)
  • holidays/countries/senegal.py (1 hunks)
  • holidays/locale/en_US/LC_MESSAGES/SN.po (1 hunks)
  • holidays/locale/fr_SN/LC_MESSAGES/SN.po (1 hunks)
  • tests/countries/test_senegal.py (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
tests/countries/test_senegal.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.
holidays/countries/senegal.py (1)
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:102-147
Timestamp: 2025-06-04T10:05:58.754Z
Learning: For Senegal holidays: Ashura (10 Muharram) in 2024 falls on July 17th, not July 16th. The user Wasif-Shahzad confirmed this date is correct for Senegal specifically, as Islamic holiday dates can vary by country based on local religious authorities and moon sighting practices.
🪛 Pylint (3.3.7)
tests/countries/test_senegal.py

[convention] 1-1: Missing module docstring

(C0114)


[convention] 19-19: Missing class docstring

(C0115)


[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSenegal.setUpClass' method

(W0221)


[convention] 26-26: Missing function or method docstring

(C0116)


[convention] 29-29: Missing function or method docstring

(C0116)


[convention] 32-32: Missing function or method docstring

(C0116)


[convention] 36-36: Missing function or method docstring

(C0116)


[convention] 40-40: Missing function or method docstring

(C0116)


[convention] 44-44: Missing function or method docstring

(C0116)


[convention] 48-48: Missing function or method docstring

(C0116)


[convention] 52-52: Missing function or method docstring

(C0116)


[convention] 56-56: Missing function or method docstring

(C0116)


[convention] 68-68: Missing function or method docstring

(C0116)


[convention] 80-80: Missing function or method docstring

(C0116)


[convention] 92-92: Missing function or method docstring

(C0116)


[convention] 104-104: Missing function or method docstring

(C0116)


[convention] 116-116: Missing function or method docstring

(C0116)


[convention] 128-128: Missing function or method docstring

(C0116)


[convention] 148-148: Missing function or method docstring

(C0116)


[convention] 163-163: Missing function or method docstring

(C0116)


[convention] 181-181: Missing function or method docstring

(C0116)

holidays/countries/senegal.py

[convention] 27-27: Line too long (147/100)

(C0301)


[convention] 1-1: Missing module docstring

(C0114)


[warning] 42-42: Keyword argument before variable positional arguments list in the definition of init function

(W1113)


[convention] 104-104: Missing class docstring

(C0115)


[convention] 108-108: Missing class docstring

(C0115)


[convention] 112-112: Missing class docstring

(C0115)

🔇 Additional comments (10)
holidays/locale/en_US/LC_MESSAGES/SN.po (1)

1-100: Well-structured English localization file.

The gettext format is properly implemented with comprehensive translations for all Senegal holidays. The metadata headers are complete and the English translations accurately represent the French holiday names.

holidays/locale/fr_SN/LC_MESSAGES/SN.po (1)

1-100: Correct source language file structure.

This French localization file properly implements the source language approach where msgid entries contain the original French text and empty msgstr entries are expected since no translation is needed from French to French.

tests/countries/test_senegal.py (4)

19-31: Good test class setup following project patterns.

The test setup properly inherits from CommonCountryTests, uses the correct year range starting from 1964 (Senegal's independence), and includes a separate instance for testing Islamic holidays without estimated dates.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 19-19: Missing class docstring

(C0115)


[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSenegal.setUpClass' method

(W0221)


[convention] 26-26: Missing function or method docstring

(C0116)


[convention] 29-29: Missing function or method docstring

(C0116)


56-66: Well-structured holiday testing following best practices.

The test correctly follows the learned pattern of testing individual holidays with specific dates for verification followed by range checking across all years. This aligns with project standards for movable holiday testing.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 56-56: Missing function or method docstring

(C0116)


128-161: Proper implementation of observed holiday testing.

The tests correctly verify both the base holiday dates and their observed equivalents, ensuring no duplicate non-observed holidays exist on observed dates. This thoroughly validates the Sunday-to-Monday observation rule.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 128-128: Missing function or method docstring

(C0116)


[convention] 148-148: Missing function or method docstring

(C0116)


163-198: Comprehensive localization testing.

Both French (default) and English localizations are properly tested with all holiday names verified for accuracy across both languages.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 163-163: Missing function or method docstring

(C0116)


[convention] 181-181: Missing function or method docstring

(C0116)

holidays/countries/senegal.py (4)

21-41: Solid class definition with proper metadata.

The class correctly inherits from multiple holiday bases and sets appropriate country metadata including localization support and the start year based on independence.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 27-27: Line too long (147/100)

(C0301)


58-102: Well-organized holiday population method.

The holidays are logically grouped and properly localized. The implementation correctly handles both Islamic and Gregorian holidays with appropriate observed date handling for Eid holidays.


112-157: Accurate Islamic holiday date implementation.

The custom Islamic holidays class properly defines Senegal-specific dates with reliable timeanddate.com sources. The dates align with the legal framework discussed in PR comments and the contributor's confirmation of Ashura 2024 being July 17th.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 112-112: Missing class docstring

(C0115)


42-57: 🧹 Nitpick (assertive)

Fix keyword argument positioning.

The islamic_show_estimated parameter should come after *args to avoid the pylint warning about keyword arguments before variable positional arguments.

-    def __init__(self, islamic_show_estimated: bool = True, *args, **kwargs):
+    def __init__(self, *args, islamic_show_estimated: bool = True, **kwargs):
⛔ Skipped due to learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2583
File: holidays/countries/niger.py:71-71
Timestamp: 2025-05-31T03:21:53.257Z
Learning: In the holidays Python library, the standard pattern for country class __init__ methods is to have keyword arguments with defaults before *args and **kwargs, such as `def __init__(self, islamic_show_estimated: bool = True, *args, **kwargs):`. This pattern is used consistently across all country implementations and should not be flagged as an issue.
🧰 Tools
🪛 Pylint (3.3.7)

[warning] 42-42: Keyword argument before variable positional arguments list in the definition of init function

(W1113)

Co-authored-by: Panpakorn Siripanich <[email protected]>
Signed-off-by: Arkadii Yakovets <[email protected]>
@arkid15r arkid15r dismissed stale reviews from KJhellico and PPsyrius via 409d61a June 10, 2025 17:12
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
holidays/countries/senegal.py (1)

134-177: 🧹 Nitpick (assertive)

Consider extending Islamic holiday dates to 2014.

Since Grand Magal of Touba starts from 2014 in the main class, extending all Islamic holiday dates to 2014 would provide more comprehensive coverage.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87ffe73 and 409d61a.

📒 Files selected for processing (1)
  • holidays/countries/senegal.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/senegal.py (4)
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:102-147
Timestamp: 2025-06-04T10:05:58.754Z
Learning: For Senegal holidays: Ashura (10 Muharram) in 2024 falls on July 17th, not July 16th. The user Wasif-Shahzad confirmed this date is correct for Senegal specifically, as Islamic holiday dates can vary by country based on local religious authorities and moon sighting practices.
Learnt from: KJhellico
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:66-110
Timestamp: 2025-06-06T14:40:31.613Z
Learning: In the holidays library, within the _populate_public_holidays method, holidays should be arranged by calendar type (Islamic holidays first, then Gregorian holidays) without additional type grouping comments. The organization by calendar type is sufficient and follows the project's established conventions.
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:104-110
Timestamp: 2025-06-05T17:22:43.057Z
Learning: In the holidays library codebase, country alias classes (like SN, SEN for Senegal) follow a consistent convention of having no docstrings and just containing "pass". This pattern is used across all countries - examples include BR/BRA for Brazil, CA/CAN for Canada, DE/DEU for Germany, etc. Static analysis tools may flag missing docstrings, but the established codebase convention is to keep these alias classes minimal.
Learnt from: KJhellico
PR: vacanza/holidays#2609
File: holidays/countries/nauru.py:149-154
Timestamp: 2025-06-09T19:50:56.019Z
Learning: In the holidays library project, never suggest adding docstrings for alias classes (like NR, NRU country code aliases that inherit from main country classes). The project deliberately omits docstrings for these simple alias classes.
🪛 Pylint (3.3.7)
holidays/countries/senegal.py

[convention] 32-32: Line too long (128/100)

(C0301)


[convention] 33-33: Line too long (170/100)

(C0301)


[convention] 34-34: Line too long (256/100)

(C0301)


[convention] 35-35: Line too long (145/100)

(C0301)


[convention] 184-184: Line too long (163/100)

(C0301)


[convention] 185-185: Line too long (188/100)

(C0301)


[convention] 1-1: Missing module docstring

(C0114)


[convention] 124-124: Missing class docstring

(C0115)


[refactor] 124-124: Too many ancestors (8/7)

(R0901)


[convention] 128-128: Missing class docstring

(C0115)


[refactor] 128-128: Too many ancestors (8/7)

(R0901)


[convention] 132-132: Missing class docstring

(C0115)


[refactor] 180-180: Too few public methods (0/2)

(R0903)

PPsyrius
PPsyrius previously approved these changes Jun 10, 2025
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 409d61a and fbe5816.

📒 Files selected for processing (1)
  • holidays/countries/senegal.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/senegal.py (4)
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:102-147
Timestamp: 2025-06-04T10:05:58.754Z
Learning: For Senegal holidays: Ashura (10 Muharram) in 2024 falls on July 17th, not July 16th. The user Wasif-Shahzad confirmed this date is correct for Senegal specifically, as Islamic holiday dates can vary by country based on local religious authorities and moon sighting practices.
Learnt from: KJhellico
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:66-110
Timestamp: 2025-06-06T14:40:31.613Z
Learning: In the holidays library, within the _populate_public_holidays method, holidays should be arranged by calendar type (Islamic holidays first, then Gregorian holidays) without additional type grouping comments. The organization by calendar type is sufficient and follows the project's established conventions.
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:104-110
Timestamp: 2025-06-05T17:22:43.057Z
Learning: In the holidays library codebase, country alias classes (like SN, SEN for Senegal) follow a consistent convention of having no docstrings and just containing "pass". This pattern is used across all countries - examples include BR/BRA for Brazil, CA/CAN for Canada, DE/DEU for Germany, etc. Static analysis tools may flag missing docstrings, but the established codebase convention is to keep these alias classes minimal.
Learnt from: KJhellico
PR: vacanza/holidays#2609
File: holidays/countries/nauru.py:149-154
Timestamp: 2025-06-09T19:50:56.019Z
Learning: In the holidays library project, never suggest adding docstrings for alias classes (like NR, NRU country code aliases that inherit from main country classes). The project deliberately omits docstrings for these simple alias classes.
🪛 Pylint (3.3.7)
holidays/countries/senegal.py

[convention] 32-32: Line too long (128/100)

(C0301)


[convention] 33-33: Line too long (170/100)

(C0301)


[convention] 34-34: Line too long (256/100)

(C0301)


[convention] 35-35: Line too long (145/100)

(C0301)


[convention] 184-184: Line too long (163/100)

(C0301)


[convention] 185-185: Line too long (188/100)

(C0301)


[convention] 1-1: Missing module docstring

(C0114)


[convention] 124-124: Missing class docstring

(C0115)


[refactor] 124-124: Too many ancestors (8/7)

(R0901)


[convention] 128-128: Missing class docstring

(C0115)


[refactor] 128-128: Too many ancestors (8/7)

(R0901)


[convention] 132-132: Missing class docstring

(C0115)


[refactor] 180-180: Too few public methods (0/2)

(R0903)

🔇 Additional comments (3)
holidays/countries/senegal.py (3)

46-48: Confirm historical coverage – start_year = 1964 may omit valid 1960-63 data

Law 63-51 came into force July 1963, but Senegal had public-holiday legislation (incl. 20 Aug & 14 Jul Independence Day variants) before 1964.
Setting start_year to 1964 prevents the library from returning any holidays for 1960-1963. Please double-check the project’s historical‐coverage policy and update the year if earlier data should be exposed.


63-66: Scope of SUN_TO_NEXT_MON rule – verify it only targets the two Eid days

kwargs.setdefault("observed_rule", SUN_TO_NEXT_MON) attaches the rule to the whole calendar, but _add_observed() is invoked only for Eid-al-Fitr and Eid-al-Adha.
If, in the future, another holiday is switched to observed mode, it will inherit the same Sunday->Monday rule automatically. This is fine if that behaviour is intended; otherwise consider a dedicated rule or an inline mapping when calling _add_observed().


124-130: Alias classes follow project convention – no docstrings needed

Keeping SN and SEN as bare subclasses aligns with the established pattern across the codebase. Good to see the convention preserved.

🧰 Tools
🪛 Pylint (3.3.7)

[convention] 124-124: Missing class docstring

(C0115)


[refactor] 124-124: Too many ancestors (8/7)

(R0901)


[convention] 128-128: Missing class docstring

(C0115)


[refactor] 128-128: Too many ancestors (8/7)

(R0901)

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @Wasif-Shahzad 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Senegal holidays
4 participants