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

ISO 8061 negative years (BCE) #118

Closed
GoogleCodeExporter opened this Issue Mar 15, 2015 · 13 comments

Comments

Projects
None yet
2 participants
@GoogleCodeExporter

GoogleCodeExporter commented Mar 15, 2015

An ISO 8061 year before the common era can be specified as negative year.  
Creating (Instant.FromUtc) with a negative year works.  And formatting with 
InstantPattern.ExtendedIsoPattern works.  

Parsing a negative year fails with InstantPattern.ExtendedIsoPattern.

What steps will reproduce the problem?

[Test]
public void Instant_IsoBceRoundtrip()
{
  var bce = Instant.FromUtc(-1194, 3, 12, 1, 2);
  var isoBce = "-1194-03-12T01:02:00Z";

  Assert.AreEqual(isoBce, InstantPattern.ExtendedIsoPattern.Format(bce));

  // The following fails!
  Assert.AreEqual(bce, InstantPattern.ExtendedIsoPattern.Parse(isoBce));
}


Original issue reported on code.google.com by makaretu on 8 Oct 2012 at 7:29

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

As per the notes:

"Currently negative absolute years cannot be parsed, but will be formatted - so 
"13 B.C." would be formatted as "-0012" using the "yyyy" format."

I propose we don't block v1 on this.

Original comment by jonsk...@google.com on 8 Oct 2012 at 9:26

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Sorry, I did not see the notes.

I feel that not being able to round-trip Instant values is a serious drawback.  
One of the basic tenets of ISO 8061 is the 'exchange of date and time-related 
data'.  If we can generate a string but then can not read it back, we have an 
issue.  Users will develop hacks to work around it.

I'm willing to look at the code and suggest a solution and then code it.


Original comment by makaretu on 8 Oct 2012 at 11:39

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

If you want to investigate the pit of despair that constitutes our parsing 
code, feel free :)

The fact that this only applies to date/time values which can't even be 
*represented* as .NET DateTime values suggests it's going to affect a 
relatively small proportion of users.

Original comment by jonsk...@google.com on 8 Oct 2012 at 11:57

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Jon, you are correct that negative years is an edge case.  But after entering 
the "pit of despair" (is this C# v Haskell Parsec), I still believe we must 
address the issue in v1.

Formatting a negative year is typically NOT for human consumption.  A negative 
year, nearly always mean that the date is not in the current era.  So, 
formatters should throw if the 'y' pattern char is used and an era is not also 
being formatted.  When an era is also being formatted then we always format the 
absolute value of the year.

The current parser that throws when a 'y' pattern encounters a negative year is 
correct!

We add a new pattern char 'E', which is an ISO 8061 expansion year.  An 
expansion year can be negative and/or more that 4 digits.  When an expansion 
year is formatted at least 4 digits are used.  If more then 4 digits are 
required then a '+' (U+002B) is prepended (in accordance with ISO 8061) when it 
is positive; otherwise, a '-' (U+002D) is prepended.  

On parsing, the expansion year pattern optionally allows '+' or '-' and at 
least 4 digits.

The Instant Extended ISO pattern then becomes: 
E'-'MM'-'dd'T'HH':'mm':'ss.FFFFFFF'Z'. And with other ISO patterns the "yyyy" 
is replaced with "E".

Have a missed anything?

Original comment by makaretu on 9 Oct 2012 at 2:40

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

There's no need to introduce anything extra. We already have y, yy, yyy etc 
meaning "absolute year" and Y, YY etc meaning "year of era". "g" and "gg" 
represent the name of the era.

So far I'm not seeing any justification for this being a v1-blocker. The 
release will be useful to 99% of developers without changing this. Fixing it 
later will require no breaking API change - it's just a format which didn't 
work before will start to work. Why would we want to delay making Noda Time 
available for a corner case which will impact so few people? That's not to say 
we don't want to address it, just that I don't want to hold up the v1 release 
for it.

Original comment by jonathan.skeet on 9 Oct 2012 at 2:45

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Ok, I agree that getting a v1 release is important and as I said this is an 
edge case.

You said that "y" is a "absolute year".  Does this mean that it always produces 
a non-negative year string? Should formatter throw if it is year value is 
negative?  

What is your view on the 'E' pattern.  I think, I can implement fairly quickly 
(1-2 days).  I think its the correct way of treating ISO dates/times.

Original comment by makaretu on 9 Oct 2012 at 3:09

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

No, absolute years are not always positive. As per the docs:

"Some calendars support multiple eras. For example, the ISO calendar supports 
the B.C. / B.C.E. and A.D. / C.E. eras. A mapping is provided between "year 
within era" and "absolute" year - where an absolute year uniquely identifies 
the date, and does not generally skip. In the ISO calendar, the absolute year 0 
is deemed to be 1 B.C. and the absolute year 1 is deemed to be 1 A.D. thus 
giving a simplified arithmetic system.

Currently negative absolute years cannot be parsed, but will be formatted - so 
"13 B.C." would be formatted as "-0012" using the "yyyy" format."

At this point I don't want to introduce a new pattern - I believe we can do 
anything we need within the current code; we just need to allow parsing of 
negative absolute years.

Original comment by jonsk...@google.com on 9 Oct 2012 at 3:20

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

mea maxima culpa - all is correct in noda time round tripping.  I will await 
your updates.

Original comment by makaretu on 9 Oct 2012 at 3:30

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Here's a patch, generated via "hg export", that allows parsing of negative 
years.

Original comment by makaretu on 10 Oct 2012 at 2:18

Attachments:

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Thanks for the patch - it's probably going to be next week before I get to it, 
but I promise I'll get there some time :)

Original comment by jonathan.skeet on 10 Oct 2012 at 6:53

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Fixed by revision 881795ce5b56 and revision 5f225180860b.

Original comment by jonathan.skeet on 19 Oct 2012 at 11:31

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Original comment by malcolm.rowe on 23 Oct 2012 at 11:54

  • Added labels: Milestone-1.0
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Mar 15, 2015

Original comment by malcolm.rowe on 10 Nov 2012 at 10:20

  • Added labels: Milestone-1.0.0
  • Removed labels: Milestone-1.0

@malcolmr malcolmr added the bug label Mar 15, 2015

@malcolmr malcolmr modified the milestone: 1.0.0 Mar 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment