stevedlawrence commented on code in PR #1685:
URL: https://github.com/apache/daffodil/pull/1685#discussion_r3417870838
##########
daffodil-core/src/main/scala/org/apache/daffodil/lib/calendar/DFDLCalendar.scala:
##########
@@ -170,8 +197,11 @@ trait ToDateMixin { self: DFDLCalendar =>
def toDate(): DFDLDate = DFDLDate(calendar.clone().asInstanceOf[Calendar],
self.hasTimeZone)
}
-case class DFDLDate(calendar: Calendar, override val hasTimeZone: Boolean)
- extends DFDLCalendar
+case class DFDLDate(
+ calendar: Calendar,
+ override val hasTimeZone: Boolean,
+ prolepticBCE: Boolean = false
Review Comment:
I'm not sure I understand the need for this proleptic flag. Doesn't the
Calendar contain all the necessary era information in the ERA field?
We also still have a number of references in our codebase to `EXTENDED_YEAR`
and patterns with `uuuu`--that feels like a bug to me. Should we instead be
setting/getting the YEAR and ERA components everywhere and using `yyyy` in all
of our patterns? And removing all references to EXTENDED_YEAR and uuuu?
So the idea is we let ICU parse things according to dfdl:calendarPattern.
When we want to create a string for the infoset or in one of the path
`fn:year-from-date*()` functions we get the YEAR and ERA fields, and if the ERA
field is BC we negate the year. Whether or not the pattern has a G in it
shouldn't matter I think?. And for unparse, we read a date from the infoset and
set the YEAR to the absolute value of the year and set the ERA to BC if the
year was negative. It is a little extra work (which it looks like you already
have logic for in a few places), but it avoids the potential of confusing
EXTENDED_YEAR with YEAR. Seems we don't really want EXTENDED_YEAR or `uuuu`
appearing anywhere in our codebase, except for tests making sure 'uuuu'
patterns work correct.
Note I think this change would mean that it's impossible to use
dfdl:calendarPattern to model BC years using `yyyy` without a G indicator
(which is maybe the original reason for switching to `uuuu` and EXTENDED_YEAR
many years ago?). But I think that's correct. Many DFDL semantics are based on
ICU, and that is just a limitation of the `yyyy` pattern. If you want a pattern
that is of the form `yyyy-MM-dd` without a `G` era indicator, and you want
negative years to indicate the era, then you must use `uuuu` with the
assumption that your years are astronomical. If your years aren't astronomical,
then DFDL can't currently model them, unless it's added a new feature to
support negative yyyy patterns.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
