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

Indian National Holidays #12

Closed
wants to merge 5 commits into from
Closed

Conversation

RohanSakhale
Copy link

This PR introduces national holidays in India referenced from Wiki Public Holidays Page

New Changes

  1. Introduced India as a country
  2. Added test case for the Indian Holidays to be verified

To ask

Many times there are state-specific holidays in many countries, we can optionally introduce state-specific holidays that can be passed as active state variables or variables when checking isHoliday this would make the repository very useful when utilizing it across countries.

@Nielsvanpach
Copy link
Member

Nielsvanpach commented Jan 17, 2024

We are open to support states or regions.

As this is also discussed on the German PR, I've added it as feature request: #14 to have a single discussion on it.

@RohanSakhale
Copy link
Author

@Nielsvanpach attempted to add region support, can you review

@lokesh-zersys
Copy link

@RohanSakhale Most of the holidays you have added will change every year, eg Diwali will be on a different date each year.
So you cannot add such holidays. Unless we come up with some logic to predict the date.

@RohanSakhale
Copy link
Author

@lokesh-zersys yes that is true, that thought was in my mind, but need to think of a way out to provide dates based on Hindu Calendar, which I was searching but couldn't found concreate way out unless we start defining them ourself in the repo

@jigar-dhulla
Copy link

jigar-dhulla commented Jan 18, 2024

In my opinion, we should add only Mandatory Holidays based on Country's and respective State's(Region) Law.
For those who won't to change the list of Holiday's, should extend the India class and override the methods as required. Yes, including Diwali, Christmas and Eid.

For e.g. according to Maharastra Law. There are only 4.

3 National Holiday

  • 26th Jan - Republic Day
  • 15th Aug - Independence Day
  • 2nd Oct - Gandhi Jayanti

1 State Holiday

  • 1st May - Labour Day / Maharashtra Day

@jigar-dhulla
Copy link

Also, list of holidays changes based on sector as well, not just region. E.g. banking and school sector will have more holidays than offices. Just a thought to keep in mind when creating a list of Holidays that is generic for all.

@Nielsvanpach
Copy link
Member

I've added documentation on how I think we should manage regions or states:
https://github.com/spatie/holidays?tab=readme-ov-file#contributing-a-new-country

Could you get this PR up to date or maybe better, start a new one?

@RohanSakhale
Copy link
Author

@Nielsvanpach updated based on documented structure

I believe we should also support region when creating country object using Holidays::for, please suggest your views

'Ram Navami' => '04-21',
'Mahavir Jayanti' => '04-25',
'Buddha Purnima' => '05-26',
'Eid al-Fitr' => '05-14',
Copy link
Member

Choose a reason for hiding this comment

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

I don't think a fixed list can be used for Islamic holidays

Copy link
Author

Choose a reason for hiding this comment

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

There we would need a manual intervention for adding Islamic and Hindu holidays based on Moon based calendar, this can either come through config or via class abstract methods

@Nielsvanpach
Copy link
Member

Thanks, but I'm closing this PR as it doesn't feel complete yet. Feel free to reopen once fixed!

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