Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/add nyse holidays #651

Merged
merged 6 commits into from
Apr 28, 2022
Merged

Conversation

nadime
Copy link
Contributor

@nadime nadime commented Apr 7, 2022

Change to add NYSE holidays as an additional "financial markets" set of holidays. These were still done within the "countries" infrastructure, but perhaps once a few more financial markets are added we should modify the structure slightly.

I've included only "full day" holidays for the NYSE historically and done my best to match with a few sources:
https://s3.amazonaws.com/armstrongeconomics-wp/2013/07/NYSE-Closings.pdf
and
https://nyseholidays.blogspot.com/2012/11/nyse-holidays-from--.html where year1/year2 are always a year ending in X1 and X0 (e.g. 1941-1950).

I've tried my best to conform to the standards I saw, for example adding " (Observed)" at the end of holidays when the observed holiday does not fall on the actual holiday, but please let me know if you see anything else you'd like changed.

Cheers!

@mborsetti
Copy link
Contributor

Very nice work!

Per https://github.com/dr-prodigy/python-holidays/issues/458, the ISO 20022 code to be used here is XNYS.

@dr-prodigy not sure how you want to categorize these so they can grow (we already have something for European markets).

@coveralls
Copy link

coveralls commented Apr 8, 2022

Pull Request Test Coverage Report for Build 2116314726

  • 175 of 175 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 98.942%

Totals Coverage Status
Change from base Build 2102842664: 0.03%
Covered Lines: 6349
Relevant Lines: 6366

💛 - Coveralls

@nadime
Copy link
Contributor Author

nadime commented Apr 8, 2022

Ah yes, of course - you're referring to the README, right (fixed there)? Or did I miss somewhere else you want the right MIC?

@nadime
Copy link
Contributor Author

nadime commented Apr 19, 2022

Are we waiting on anything here? Can we merge this into beta?

dr-prodigy added a commit that referenced this pull request Apr 28, 2022
…al() method (dr-p)

Support for NY Stock Exchange #651, #458 (nadime, dr-p)
@dr-prodigy dr-prodigy merged commit 4500fc5 into vacanza:beta Apr 28, 2022
@dr-prodigy
Copy link
Collaborator

Hi @nadime thanks for your great work!
I in fact decided to apply some refactoring to allow for a better financial markets integration, adding a dedicated package (financial), adding a specific method for retrieving those classes only (list_supported_financial()) and reviewing tests accordingly.
I also added another synonym for the new class (XNYS) as suggested by @mborsetti.
Merged in beta, thx! 👍

@dr-prodigy dr-prodigy mentioned this pull request Apr 28, 2022
dr-prodigy added a commit that referenced this pull request Jun 5, 2022
Version 0.14
============

Released June 5, 2022

- Drop support for EOL Python 3.6 #328 (hugovk, dr-p)
- Package review #662 (dimbleby)
- Added financial markets support: ECB and NYSE, list_supported_financial() method (dr-p)
- Support for NY Stock Exchange #651, #458 (nadime, dr-p)
- Support for Malta #612, #630 (rafelbev)
- Support for Madagascar #656 (fav007)
- Support for Cyprus #410, #665 (digidestination, avnigo)
- Ireland as standalone country #636, #639 (TeoTN, dr-p, javicalle)
- Australia fixes #631 (jeremychrimes)
- Singapore updates #652 (mborsetti)
- Saudi Arabia fixes #642 (OsaydAbdu)
- Spain fixes #634 (javicalle)
- US fixes #648 (dashdrum)
- Greece fixes #659 (tudorvaran)
- India doc fixes #657 (dr-p)
- Poland fix #663 (kfsz)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants