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

Add Date::new_from_iso(date_iso, X) example for each calendar X #1844

Conversation

andrewpollack
Copy link
Contributor

@andrewpollack andrewpollack commented May 3, 2022

Adding a Date::new_from_iso(date_iso, X) example at the top level of each calendar.

  • Skipped Japanese, leaving until future update when I understand examples for it better

---Below issue has been resolved. Thank you!---
Separately, I am running into issues with the Coptic calendar. Is there a missing permission on access to the type? To demonstrate, the following works:

use icu::calendar::{Date, DateTime, indian::Indian};

let date_iso = Date::new_iso_date_from_integers(1970, 1, 2).unwrap();
let date_indian = Date::new_from_iso(date_iso, Indian);

But the following does not work:

use icu::calendar::{Date, DateTime, coptic::Coptic};

let date_iso = Date::new_iso_date_from_integers(1970, 1, 2).unwrap();
let date_coptic = Date::new_from_iso(date_iso, Coptic);

With the error:

error[E0423]: expected value, found struct `Coptic`
let datetime_coptic = DateTime::new_from_iso(datetime_iso, Coptic);
                                                           ^^^^^^ constructor is not visible here due to private fields

@andrewpollack andrewpollack requested a review from a team as a code owner May 3, 2022 16:00
@Manishearth
Copy link
Member

For Coptic you have to call Coptic::new() for now, it's marked as non_exhaustive so it can't be constructed the usual way.

@andrewpollack
Copy link
Contributor Author

Thanks for the followup @Manishearth !

I went ahead and added the Coptic examples, along with the Buddhist examples. This PR is now in a stable state to review when ready

@Manishearth
Copy link
Member

Also i just realized Coptic should not be non_exhaustive, that's a bug. I removed that bit in my other PR.

The thing with Japanese is that it has era data that changes occasionally, so we need to load it. Typically the thing to do in tests is to construct it with a let provider = icu_testdata::get_provider();.

@andrewpollack
Copy link
Contributor Author

Adjusted the Coptic example with your changes, thanks @Manishearth !

Separately, I'm still running into issues getting the Japanese example working, so I'll work that into a separate PR/discussion. More specifically, getting icu_testdata properly into the Cargo.toml and running in the doctest environment is running into compiler errors that I have not seen before:

thread 'rustc' panicked at 'assertion failed: !new_self_ty.has_escaping_bound_vars()', compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs:1688:9
...
query stack during panic:
#0 [typeck] type-checking `main::_doctest_main_src_japanese_rs_7_0`
#1 [typeck_item_bodies] type-checking all item bodies
#2 [analysis] running analysis passes on this crate
end of query stack
error: internal compiler error: trimmed_def_paths constructed

@Manishearth
Copy link
Member

Manishearth commented May 5, 2022

Ah, hmm, we've been getting that error a couple times now, it's rust-lang/rust#96223. It just got fixed, I'll try to get it uplifted to beta.

Make sure that you're importing icu_provider with the "serde" feature as a dev-dependency, that ICE is confusing but it essentially points to a missing trait impl, usually in our case a Deserialize one,

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.

2 participants