-
Notifications
You must be signed in to change notification settings - Fork 5
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
Guess Julian calendar for dates before 1582. #66
Conversation
Bug: T75272
@@ -130,6 +131,11 @@ public function validInputProvider() { | |||
'+2000000000000000-00-00T00:00:00Z', | |||
TimeValue::PRECISION_Ga, | |||
), | |||
'-2000000000000000-00-00T00:00:00Z' => array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it's a good idea to mark dates millions of years before BC as Julian. There should be a lower limit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking them as Gregorian doesn't make any more sense. It just doesn't matter. So, for consistency, it's easier to just assume anything before 1582 is Julian.
Astronomers use Julian years for dates in the deep past, as far as I know.
This is not consistent to what our JavaScript UI did previously, see: |
There is no need for this to exactly emulate the heuristics previously used by the UI. In particular, making the calendar model depend on the precision makes no sense to me. I can only see this leading to inconsistencies and surprises. E.g. we could decide to highlight "old" (proleptic) Gregorian dates in the UI, because they are unusual. But we'd have to again take into account the precision. Is see no good reason to do this, it just introduces complexity. One question is whether we should assume Julian for years < 1582 or < 1583. The switchover date was in October... |
After some discussion with Lydia, it seems that we should put the switchover to 1583 instead of 1582, since the actual switch was in October, closer to 1583-01-01. |
* | ||
* @return string | ||
* - if $timeParts[7] is set, use $this->calendarModelParser to parse it into a URI. | ||
* - otherwise, if $this->getOption( self::OPT_CALENDAR ) is set not null, return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"set not null"?
31047ce
to
28e9b81
Compare
Refers to changes introduced by https://gerrit.wikimedia.org/r/#/c/209247/
@thiemowmde Thanks for the patch, do you think we can merge this now? |
@@ -48,7 +49,7 @@ public function __construct( | |||
) { | |||
parent::__construct( $options ); | |||
|
|||
$this->defaultOption( self::OPT_CALENDAR, self::CALENDAR_GREGORIAN ); | |||
$this->defaultOption( self::OPT_CALENDAR, null ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: We can set this option to CALENDAR_GREGORIAN
in Wikibase.git, if we need to avoid a breaking change.
Guess Julian calendar for dates before 1582.
The calendar should be shown if it's different from the one that the parser would guess, as of wmde/Time#66 Bug: 75272 Change-Id: Ia0d7b1548972b6f81538c8f5182f8e014bfc1a36
See https://phabricator.wikimedia.org/T75272