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

Fix Persian/Gregorian calendar conversion #6118

Merged
merged 2 commits into from
Mar 28, 2024
Merged

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Mar 28, 2024

Closes #5960, closes #3757, closes #6077, closes #5946, closes #5062.

This fixes two issues with the Persian to Gregorian and Gregorian to Persian date conversion.

  1. Dates were sometimes off by one in months greater than 6, which resulted in 30 being displayed as the first day of the month.
  2. The leap year calculation wasn't quite right, resulting in an incorrect number of days in the month.

This ports the code from ICU to perform the conversion and adds tests from all of the issues/PRs that have been raised for this issue.

Test instructions

  1. Open calendar stories
  2. Change calendar system to Persian
  3. Navigate forward/backward by month and verify that months always start with 1, not 30 or 31.
  4. Navigate to "Farvardin 1404 AP", and verify that the month starts on a Friday.

@devongovett devongovett changed the title Fix Persian to Gregorian calendar conversion Fix Persian/Gregorian calendar conversion Mar 28, 2024
@rspbot
Copy link

rspbot commented Mar 28, 2024

@rspbot
Copy link

rspbot commented Mar 28, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Math checks out as equivalent to that in https://github.com/unicode-org/icu/blob/main/icu4j/main/core/src/main/java/com/ibm/icu/util/PersianCalendar.java#L422-L445 and the other helper functions in that file. Also verified the problematic cases with 12/22/2022 (and other dates adjacent) being computed to the wrong equivalent Persian date and that the case where Esfand 1403 AP (and other months adjacent) had the wrong start date value no longer happen. Also verified that 1404/1/1 to be on a Friday (made in #6077 (comment)) is reflected in the calendar too.

@devongovett devongovett merged commit 784737e into main Mar 28, 2024
27 checks passed
@devongovett devongovett deleted the fix-persian-calendar branch March 28, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants