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

Swap Adar I and Adar II in Hebrew #4

Closed
gregarkhipov opened this issue Dec 5, 2015 · 4 comments
Closed

Swap Adar I and Adar II in Hebrew #4

gregarkhipov opened this issue Dec 5, 2015 · 4 comments

Comments

@gregarkhipov
Copy link

Hello, a nice work you've done here!

I wanted to use this tool for our python-powered site and noticed that Adar months in leap year are not where they're supposed to be. The thing is that in Hebrew calendar Adar I is additional. I.e., a person born in Adar in a common Hebrew year would celebrate his birthday in Adar Bet (the second) in a leap year. Same thing with the Holidays.

So this way, hebrew.from_gregorian() should return consequently ... — 10 — 11 — 13 — 12— 1 — ... for a leap year. It might seem irrational, but this is the way Hebrew calendar works.

Since your package is on PyPI, I realize that this change will result in existing apps using this library not working properly when they update the library. But I assume nobody yet used it for Hebrew, because they would definitely notice the issue.

Although, I could make a pull request, but later, when I'm less busy.

@fitnr
Copy link
Owner

fitnr commented Dec 5, 2015

Thanks for bringing this up. I think the converter is currently working in the expected fashion. At least, it agrees with every other converter and joint listing I can find (1, 2, 3)

In embolismic years, the 12th month is Adar I, and the 13th is Adar II, so it seems appropriate for hebrew.from_gregorian() to return 12 and 13 respectively. If you would like to track events that usually occur in the 12th month, but sometimes in the 13th (e.g. Purim), I suggest using hebrew.leap() to check if it is a leap year.

If I've misunderstood what you're asking, it would be helpful to see a test case that the library would currently fail.

@gregarkhipov
Copy link
Author

I already adapted it for our project via using hebrew.leap().

And of course, for computing purposes Adar I is logically the 12th month, but I judge it from halachic point of view. And also my code could have been simpler if the months was swapped. Because to calculate birthdays and Holidays I have to either manually swap 12 and 13 for leap year, or redirect 12th month's events to 13th month anyways. Although, I would still need to use leap() method to set name for 12th month (Adar or Adar II), but it feels more natural (It remains the same month).

Maybe if chabad.org call a leap year's 12th month Adar I for their converter, then it's OK. But I couldn't find it out from your reference. What exactly did you mean?

@fitnr fitnr closed this as completed Dec 6, 2015
@fitnr
Copy link
Owner

fitnr commented Dec 6, 2015

I understand that it's a pain to deal with leap months, but I think the current situation is the most general, easiest-to-understand solution.

Chabad.org doesn't explicitly list Adar I as the 12th month, but it's the 12th month that occurs in their dual-system calendar for AM 5776.

Glad you're finding the library (at least somewhat) useful!

@gregarkhipov
Copy link
Author

I didn't state that consequently Adar I is 13'th month, I know it's 12th, if you count from Nissan. I just said it's more profound to make it 13th, even from OOP point of view (since Adar and Adar II have the same events). BTW, additional Adar comes explicitly before the common Adar, unlike in other calendars, so there's no comparison. But I'm cool with the current state, if you don't wanna touch it. It still serves for good things.

Happy Chanukah!

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

No branches or pull requests

2 participants