Add Bombay Stock Exchange (BSE) holidays#3170
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a Bombay Stock Exchange (XBOM / BSE) financial calendar subclassing NationalStockExchangeOfIndia, registers and exports it, adds locale PO templates and unit tests, and makes parent-translation fallback lookup more robust. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (6)
🧰 Additional context used🧠 Learnings (50)📓 Common learnings📚 Learning: 2025-06-28T10:39:19.185ZApplied to files:
📚 Learning: 2025-06-29T09:37:35.283ZApplied to files:
📚 Learning: 2025-03-05T17:51:00.633ZApplied to files:
📚 Learning: 2025-06-10T12:17:58.880ZApplied to files:
📚 Learning: 2025-06-26T15:34:35.476ZApplied to files:
📚 Learning: 2025-06-11T18:32:25.595ZApplied to files:
📚 Learning: 2025-03-30T13:33:31.598ZApplied to files:
📚 Learning: 2025-05-10T04:34:02.406ZApplied to files:
📚 Learning: 2025-06-06T15:22:39.950ZApplied to files:
📚 Learning: 2025-05-09T18:34:33.990ZApplied to files:
📚 Learning: 2025-03-08T11:28:48.652ZApplied to files:
📚 Learning: 2025-03-30T18:25:07.087ZApplied to files:
📚 Learning: 2025-08-26T20:10:05.288ZApplied to files:
📚 Learning: 2025-05-06T15:25:44.333ZApplied to files:
📚 Learning: 2025-09-07T19:28:19.153ZApplied to files:
📚 Learning: 2025-09-26T13:58:49.363ZApplied to files:
📚 Learning: 2025-06-25T10:09:29.029ZApplied to files:
📚 Learning: 2025-04-17T17:08:48.082ZApplied to files:
📚 Learning: 2025-03-31T20:25:12.808ZApplied to files:
📚 Learning: 2025-04-13T19:11:32.337ZApplied to files:
📚 Learning: 2025-09-26T13:44:12.652ZApplied to files:
📚 Learning: 2025-06-10T05:08:07.939ZApplied to files:
📚 Learning: 2025-09-06T20:47:29.638ZApplied to files:
📚 Learning: 2025-08-30T12:52:58.539ZApplied to files:
📚 Learning: 2025-03-31T11:50:50.488ZApplied to files:
📚 Learning: 2025-05-10T04:02:13.815ZApplied to files:
📚 Learning: 2025-12-17T14:46:04.977ZApplied to files:
📚 Learning: 2025-08-08T14:37:03.045ZApplied to files:
📚 Learning: 2025-08-20T19:46:15.625ZApplied to files:
📚 Learning: 2025-08-28T21:03:22.954ZApplied to files:
📚 Learning: 2025-08-09T18:31:23.218ZApplied to files:
📚 Learning: 2025-04-03T16:58:27.175ZApplied to files:
📚 Learning: 2025-08-08T14:37:03.045ZApplied to files:
📚 Learning: 2025-08-21T05:56:33.276ZApplied to files:
📚 Learning: 2025-07-10T21:08:23.709ZApplied to files:
📚 Learning: 2025-06-18T10:07:58.780ZApplied to files:
📚 Learning: 2025-08-26T21:24:41.827ZApplied to files:
📚 Learning: 2025-06-25T20:55:00.642ZApplied to files:
📚 Learning: 2025-06-16T11:46:35.303ZApplied to files:
📚 Learning: 2025-05-10T04:35:01.221ZApplied to files:
📚 Learning: 2025-06-16T11:46:35.303ZApplied to files:
📚 Learning: 2025-06-18T10:18:59.447ZApplied to files:
📚 Learning: 2025-08-25T10:51:08.068ZApplied to files:
📚 Learning: 2025-03-19T16:54:58.657ZApplied to files:
📚 Learning: 2025-06-19T02:34:18.382ZApplied to files:
📚 Learning: 2025-08-11T13:48:45.953ZApplied to files:
📚 Learning: 2025-03-04T11:32:45.095ZApplied to files:
📚 Learning: 2025-03-04T11:32:45.095ZApplied to files:
📚 Learning: 2025-09-17T15:53:16.940ZApplied to files:
🧬 Code graph analysis (2)holidays/financial/bombay_stock_exchange.py (1)
holidays/financial/__init__.py (1)
🔇 Additional comments (6)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Bombay Stock Exchange (BSE) with market code XBOM to the holidays library. Since BSE shares an identical holiday calendar with the National Stock Exchange (NSE) due to SEBI regulation, the implementation uses inheritance to avoid code duplication while maintaining proper directory structure.
Key Changes
- New
BombayStockExchangeclass that inherits fromNationalStockExchangeOfIndia - Comprehensive test suite covering all holidays and localizations (en_IN, en_US, gu, hi)
- Registry entry with aliases (XBOM, BSE)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| holidays/financial/bombay_stock_exchange.py | New implementation that inherits from NSE, cleverly reuses XNSE translations via _init_translation override |
| tests/financial/test_bombay_stock_exchange.py | Comprehensive test suite covering all holidays, observed/non-observed dates, and localizations |
| holidays/registry.py | Added registry entry for bombay_stock_exchange with XBOM and BSE aliases |
| README.md | Documentation table entry for Bombay Stock Exchange with supported languages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@KJhellico sir , I've pushed all the requested fixes in commit f6d941e. However, the PR diff is still showing the old files. I'm not sure if this is a GitHub glitch or if I made a mistake while pushing. Could you please check the commit directly to verify the changes? Thanks! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #3170 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 306 307 +1
Lines 18288 18298 +10
Branches 2336 2336
=========================================
+ Hits 18288 18298 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@KJhellico sir , I noticed something about the .po file generation. Currently, the script creates a pretty bare header, so I had to manually add standard fields like Project-Id-Version, Language-Team, MIME-Version, and Content-Type to ensure it matched the existing files. IMO, we should update the generation script (scripts/l10n/generate_po_files.py) to automatically populate these technical fields with standard defaults (setting UTF-8, pulling the version from the package, setting the default Team, etc.). It would make the process much smoother for future contributors so they only need to fill in their Name/Email |
Yes, that would be helpful. |
Co-authored-by: ~Jhellico <[email protected]> Signed-off-by: Paresh Joshi <[email protected]>
Signed-off-by: Paresh Joshi <[email protected]>
|
|
Thank you @KJhellico and @PPsyrius for the review and approval! |
arkid15r
left a comment
There was a problem hiding this comment.
LGTM
@pareshjoshij thanks for both -- the idea and implementation.



Proposed change
This PR adds support for the Bombay Stock Exchange (BSE), market code
XBOM.Closes #3149
Implementation Details:
As discussed in the linked issue, the BSE shares an identical holiday calendar with the National Stock Exchange (NSE) as both are regulated by SEBI.
Following the feedback from @arkid15r regarding structural consistency and long-term perspective, I have implemented this as a separate class that inherits from
NationalStockExchangeOfIndia, rather than just adding an alias. This ensures the correct directory structure while avoiding code duplication for the holiday logic.Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.