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

Coptic and Ethiopic date conversions from iso fail for dates sufficiently in the past #2254

Closed
Manishearth opened this issue Jul 26, 2022 · 3 comments · Fixed by #2714
Closed
Assignees
Labels
C-calendar Component: Calendars T-bug Type: Bad behavior, security, privacy

Comments

@Manishearth
Copy link
Member

    #[test]
    fn test_coptic_regression() {
        let iso_date = Date::new_iso_date(-100, 3, 3).unwrap();
        let coptic = iso_date.to_calendar(Coptic);
    }

This fails with:

---- coptic::tests::test_coptic_regression stdout ----
thread 'coptic::tests::test_coptic_regression' panicked at 'called `Result::unwrap()` on an `Err` value: TryFromIntError(())', components/calendar/src/coptic.rs:196:37
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::result::unwrap_failed
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1785:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/result.rs:1078:23
   4: icu_calendar::coptic::Coptic::fixed_from_coptic_integers
             at ./src/coptic.rs:196:20
   5: icu_calendar::coptic::Coptic::coptic_from_fixed
             at ./src/coptic.rs:207:30
   6: <icu_calendar::coptic::Coptic as icu_calendar::calendar::Calendar>::date_from_iso
             at ./src/coptic.rs:104:9
   7: icu_calendar::date::Date<A>::new_from_iso
             at ./src/date.rs:112:21
   8: icu_calendar::date::Date<A>::to_calendar
             at ./src/date.rs:125:9
   9: icu_calendar::coptic::tests::test_coptic_regression
             at ./src/coptic.rs:312:22
  10: icu_calendar::coptic::tests::test_coptic_regression::{{closure}}
             at ./src/coptic.rs:310:5
  11: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
  12: core::ops::function::FnOnce::call_once
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

What's happening is that month in coptic_from_fixed is ending up negative.

The code we have does seem to match the code from the book, I suspect what's happening is that dates before the Julian epoch are messing up the math somehow. I haven't been able to figure out what's wrong though.

Worth investigating.

@Manishearth Manishearth added T-bug Type: Bad behavior, security, privacy C-calendar Component: Calendars labels Jul 26, 2022
@Manishearth
Copy link
Member Author

cc @pandusonu2 would you have time to look into this?

@pandusonu2 pandusonu2 self-assigned this Jul 27, 2022
@sffc
Copy link
Member

sffc commented Jul 30, 2022

@Manishearth Can you assign this issue to a milestone, such as 1.x Untriaged?

@Manishearth Manishearth added this to the ICU4X 1.x Untriaged milestone Jul 30, 2022
@Manishearth
Copy link
Member Author

Potentially fixable with #2704

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-calendar Component: Calendars T-bug Type: Bad behavior, security, privacy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants