-
-
Notifications
You must be signed in to change notification settings - Fork 196
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
Add Moroccan Holidays #56
Conversation
* occur based on the lunar calendar sequence. The order listed reflects the chronological occurrence | ||
* of these holidays throughout the year. | ||
*/ | ||
return [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I have the right to use geniusts/hijri-dates
to convert Date convert Gregorian date to Hijri date and vice versa ?
i will try to test it today to check how much it's accurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a necessary feature to not exclude half the holidays from Muslim countries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not really familiar with this. Could you shortly explain what this package would do and how we could benefit from it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The geniusts/hijri-dates package is a reliable PHP library designed to facilitate the conversion between Gregorian and Hijri dates. In our case, it can be incredibly useful for handling Islamic holidays that are based on the Hijri calendar.
The Islamic calendar follows a lunar system, which means that its months and years are determined by the moon's phases. This leads to variations in the lengths of months and the occurrence of Islamic holidays. By integrating this package, we gain the ability to accurately convert dates between the Gregorian and Hijri calendars.
I am currently in the process of testing the package to ensure its efficacy. I have a slight doubt about its accuracy, but my friend from the Maldives has used it in his PR#63. I will conduct thorough testing today to evaluate its performance and accuracy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Nielsvanpach,
I utilized the geniusts/hijri-dates package to compute Islamic holidays for Morocco. I have included tests for the years 2023, 2024, 2025, and 2026. The results match the date converter provided by the Minister of Religious Endowments and Islamic Affairs.
Please note that it's not possible to predict Islamic holidays with 100% certainty. Ministry observers use the naked-eye observation of the New Moon at the end of each lunar month to determine if the current month is 29 or 30 days, impacting the declaration of the new month.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for that! Just for clarity; do countries with Islamic public holidays all have the same dates for them? So is Eid al-Fitr
celebrated everywhere on the same day, or are there exceptions?
If so, is it possible to not limit this to only Morocco, but make the calculations accessible for all countries with Islamic holidays? I would prefer a single implementation for those calculations instead of it being scattered all around the codebase for different countries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nielsvanpach It is common for Islamic countries but the phase of the moon is not the same everywhere; For example Eid Al-Fitr
may occur on the 10th of April for Algeria and on the 11th of April for Morocco.
Also, the duration of days off may differ between countries, Some may take a week and some may take 2 days.
Btw, this #90 is also making use of the same package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The topic is more intricate than it seems, and there are ongoing debates in Islamic countries. There are two main differences between countries:
-
Observation Methods: Not all countries share the same date due to differences of approximately ±1 day. This is because each country employs its own method to observe the new moon, whether through naked-eye observation, telescopes, or astronomical calculations. Some countries find it impossible to determine the new moon using the naked eye.
As an illustration, consider the current Hijri month and the next one. If you check the Hijri date today in all Islamic countries, it will be 09/07/1445. To understand why they all have the same date, check this , representing the visibility of the moon on the first day of the current Hijri month "Rajab" (source).
In this image, all countries easily observe the new moon with the naked eye, leading them to declare the 13th of January 2024 as the first day of the month "Rajab."
Now, observe the visibility of the moon on the first day of the next Hijri month in (source).
You can see that only the countries of West Africa (Morocco, Algeria, etc.) can easily observe the new moon. They declare the 11th of February 2024 as the first day of the month "Shaban." However, Egypt and other Middle Eastern countries may face challenges in seeing the new moon. Consequently, some will declare February 11, 2024, as the first day based on astronomical calculations, while others will wait until February 12, 2024.
To address these differences, I used an adjustment of
-1
day to make the calculations more accurate for Morocco.Hijri::setDefaultAdjustment(-1);
-
Holiday Duration: Another difference is the number of holidays and the number of days taken as holidays. For example, "Eid al-Fitr" is observed for just 2 days in Morocco, but it's 3 days in Algeria and 4 days in Saudi Arabia.
In conclusion, having the same date for all countries is challenging, and achieving 100% accuracy in calculations is difficult because each country has its own protocol for the observation of the new moon.
PS: I fixed the PHPStan failure.
I've created a new PR #186, (originally #63), in which I've added a Hijri Calendar trait that we can use geniusts/hijri-dates to calculate Hijri dates. |
composer.json
Outdated
"nesbot/carbon": "^2.72.1", | ||
"ext-calendar": "*" | ||
"ext-calendar": "*", | ||
"geniusts/hijri-dates": "^1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're not using this library for following reasons:
…grated the calculation function from the Ministry of Endowments and Islamic Affairs.
0d92669
to
dc812c7
Compare
…unit test coverage for the new holiday addition
@Nielsvanpach This change simplifies the codebase and reduces external dependencies, thank you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your efforts on this!
|
||
public function defaultLocale(): string | ||
{ | ||
return 'en'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be Arabic by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanx for the review
In fact, I assumed that the Package would be used globally and therefore set English as the default language.
/** @return array<string, CarbonImmutable> */ | ||
protected function variableHolidays(int $year): array | ||
{ | ||
// Calculate the current Hijri year based on the Gregorian year |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have a look at other Islamic countries and how their lunar based holidays are resolved. For example: Turkey, Bahrein and Egypt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed the other contributions, and it seems they've hardcoded the expected dates of Islamic holidays. Personally, I don't find that approach ideal. Instead, I opted to use a date converter for more dynamism. While it may not be 100% accurate, it offers flexibility. If you still prefer the hardcoded holiday expectation dates, I'm willing to make the change tonight.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This pull request enhances the support for Moroccan national holidays
Note:
Do I have the right to use geniusts/hijri-dates to convert Date convert Gregorian date to Hijri date and vice versa?
I will try to test it today to check how much it's accurate