search

vacanza / holidays

Generate and work with holidays in Python
https://pypi.org/project/holidays
MIT License
1.45k stars 460 forks source link

Add `UNOFFICIAL` category for the United States #1669

Closed PPsyrius closed 7 months ago

PPsyrius commented 8 months ago

Proposed change

This PR implements additional holidays (marked as UNOFFICIAL category), which includes:

Closes #1643 .

Type of change

Checklist

coveralls commented 8 months ago

Pull Request Test Coverage Report for Build 8106192363

Details

Totals Coverage Status
Change from base Build 8105363435: -0.007%
Covered Lines: 10592
Relevant Lines: 10592
💛 - Coveralls
PPsyrius commented 8 months ago

Note: due to the current United States implementation, _populate_subdiv_holidays(self) will likely throw additional holidays into all test_[category_name]_holiday. I did try changing the former to _populate_subdiv_public_holidays(self), but that doesn't work either as the current syntactic sugar interprets public as a SUBDIV name.

KJhellico commented 8 months ago

While we don't have NON_PUBLIC subdiv holidays, we can just add

        if PUBLIC not in self.categories:
            return None

to the beginning of _populate_subdiv_holidays().

When such holidays appear, we should change it to

       if PUBLIC in self.categories:
            # MLK Day ...
KJhellico commented 8 months ago

Happy Groundhog Day! 🎉😉

PPsyrius commented 8 months ago

Happy Groundhog Day! 🎉😉

It turns out from some checks they do celebrate this in Hawaii, too, instead of just the Continental U.S. huh, even if they didn't have any groundhogs there 👀

PPsyrius commented 7 months ago

If there any additional changes required for this PR? 👀

arkid15r commented 7 months ago

If there any additional changes required for this PR? 👀

No, not really. I was just thinking about NON_PUBLIC vs UNOFFICIAL naming. To me non-public is closer to private than unofficial. I know I kind of approved it in the issue discussion earlier. @KJhellico what's your opinion?

KJhellico commented 7 months ago

I also like UNOFFICIAL better. :)

sonarcloud[bot] commented 7 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

PPsyrius commented 7 months ago

I think my bad habits of git pull --rebase then git push -f for my own private repos might have accidentally been used here..., sorry about that one 💀

Anyway, I've switched all instances of NON_PUBLIC to UNOFFICIAL now. @KJhellico @arkid15r

mborsetti commented 7 months ago

Apologies, thanks for all the work on this library and this looks neat, but is there any documentation on how to use it?

I don't seem to find instructions on how to request "categories" in the documentation, yet I presume that any changes would have to be thoroughly documented to the user before a PR is merged. Where should I look?

Also, what is the definition and criteria for UNOFFICIAL (this seems to be missing from the documentation as well)? Would National Fruitcake Toss Day be included in this library or not? If so, why?

PPsyrius commented 7 months ago

@mborsetti I'm pretty sure they're listed in https://python-holidays.readthedocs.io/en/latest/api.html#holidays.holiday_base.HolidayBase

So, for example, calling up the Californian-specific list for 2024 with UNOFFICIAL list included would be 

holidays.country_holidays("US", subdiv="CA", years=2024, categories=("public","unofficial",))

We might want to update categories=None to categories=PUBLIC for default ones as listed in https://github.com/vacanza/python-holidays/blob/dev/holidays/holiday_base.py unless I misunderstood our own codebase @KJhellico @arkid15r

In terms of criteria, there are a bunch of similarities with the existing WORKDAY category - although those are officially observed by their respective governments, for which "Cinco de Mayo" would've fallen in this category due to its US Congress concurrent resolution status.

UNOFFICIAL, on the other hand, are popular days that are widely observed by citizens of a particular jurisdiction with no official observance - for which "Valentine's Day", "Groundhog Day", and "Halloween" perfectly fit.

Unless "National Fruitcakes Toss Day" is celebrated with some events being held for it, I don't think it belongs in this category except for perhaps Manitou Springs, Colorado if we ever get to add that subdivision in once #1713 is implemented.

arkid15r commented 7 months ago

Hey Mike, long time no see!

It's great you're still keeping an eye on the PH. I appreciate your reminder regarding keeping the docs up to date (which is not always the case).

Let's look into your concerns in details:

I don't seem to find instructions on how to request "categories"

The usage of categories mentioned here and in the Holiday categories support section. All supported categories specified on per entity basis and listed in the Available Countries table (Supported Categories column).

What documentation is missing is description of each category (I'll create an issue to address that). This leads us to your next concern

what is the definition and criteria for UNOFFICIAL

As there is no standard for holiday categories I trust other fellow contributors/maintainer with their choice unless I clearly notice something is not fit. Normally we resolve this kind of concerns during code reviews or in issues.

My general observation is that PH could use a better implemented/documented code ratio. Neither of the current maintainers are native English speakers. But if you want to blame somebody for that -- it'd be me. I'm responsible for this level of documentation quality (normally I review and approve every PR merged into dev/main branch -- btw, we moved to the new branch naming recently!). You can see @PPsyrius's response above with his idea of UNOFFICIAL hoiiday meaning. I can't agree more that each category purpose should be properly documented.

I also think it's worth to mention that I've started planning PH v1 roadmap recently. It has a separate issue for better documentation. If you have any suggestions/ideas please contribute.

arkid15r commented 7 months ago

We might want to update categories=None to categories=PUBLIC for default ones as listed in https://github.com/vacanza/python-holidays/blob/dev/holidays/holiday_base.py unless I misunderstood our own codebase @KJhellico @arkid15r

It's handled here. The default category is PUBLIC and designed to be modifiable for better flexibility.

mborsetti commented 7 months ago

Hey Arkadii,

Thanks for your response, and great leadership!

I do get notices of new releases and when I have time check in on all the great improvements; wonderful job, I can't believe how many countries/subdivisions are supported now!

I also think it's worth to mention that I've started planning PH v1 roadmap recently.

Wow, wonderful to see; the API does indeed need rethinking IMHO. I see if I have time to do a deeper dive.

arkid15r commented 7 months ago

I do get notices of new releases and when I have time check in on all the great improvements; wonderful job, I can't believe how many countries/subdivisions are supported now!

That's right! A lot of people contributed to PH's success. However, personally I'm really proud of the quality of the data provided by PH. @KJhellico and @PPsyrius have been doing amazing work on hunting down every single holiday in their contributions.