Skip to content
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions components/calendar/src/cal/hijri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,18 @@ pub trait Rules: Clone + Debug + crate::cal::scaffold::UnstableSealed {

/// [`Hijri`] [`Rules`] based on an astronomical simulation for a particular location.
///
/// These simulations are unofficial and are known to not necessarily match sightings
/// on the ground. Unless you know otherwise for sure, instead of this variant, use
/// [`UmmAlQura`], which uses the results of KACST's Mecca-based calculations.
/// The simulations use the relative positions of the Earth, moon, and sun to predict the
/// exact moment a new moon occurs. Because this is rarely the instant when a crescent
/// sighting occurs, the month start dates preducted by these rules will often be one or
/// more days earlier than actually observed. Applications using these rules should have
Copy link
Member

Choose a reason for hiding this comment

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

maybe say something like "These Rules shouldn't be used directly, but can form the basis for a Rules implementation based on human sightings". The correction should be applied at the Rules level, not at the Date level (but I don't really see the point of using this if you can just write a sighting Rules).

Copy link
Member Author

Choose a reason for hiding this comment

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

I made it say:

These rules can form the basis of a custom [Rules] implementation that includes data based on human sightings. As discussed in the [Hijri] documentation, using these rules without allowing for adjustments will produce dates that are only approximations of the ground truth.

/// a method for adjusting the month start date based on human sightings.
///
/// As floating point arithmetic degenerates for far-away dates, this falls back to
/// the tabular calendar at some point.
/// If you don't have a way to inject human sighting adjustments, you should probably use
/// [`UmmAlQura`], which uses the results of KACST's Mecca-based calculations and matches
/// ground truth in Saudi Arabia.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this recommendation is correct. You use UAQ if you need UAQ, tabular if you need tabular, and observational if you need observational (whenever that is). If you need observational and don't have sightings, UAQ is not an alternative.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm happy to remove this whole paragraph

///
/// The simulations are pre-computed for Gregorian years 1900 to 2140, falling back to
/// a tabular approximation outside that range.
///
/// The precise behavior of this calendar may change in the future if:
/// - We decide to tweak the precise astronomical simulation used
Expand Down