oblac / jodd-util

Essential Java utilities.
https://util.jodd.org
BSD 2-Clause "Simplified" License
39 stars 9 forks source link

DateTimeException when calling JulianDate.toLocalDateTime for certain julian days #21

Open zabetak opened 1 month ago

zabetak commented 1 month ago

Minimal code to reproduce the problem.

public static void main(String[] args) {
    int start = JulianDate.of(LocalDate.of(0, 1, 1)).getInteger();
    int end = JulianDate.of(LocalDate.of(9999, 12, 31)).getInteger();
    for (int i = start; i <= end; i++) {
      JulianDate date = new JulianDate(i, 0.0);
      try {
        date.toLocalDateTime();
      } catch (Exception e) {
        System.out.println("Problem converting Julian day " + i);
      }
    }
  }

A sample stack trace is shown below.

java.time.DateTimeException: Invalid date 'February 29' as '1500' is not a leap year
    at java.time.LocalDate.create(LocalDate.java:429)
    at java.time.LocalDate.of(LocalDate.java:269)
    at java.time.LocalDateTime.of(LocalDateTime.java:336)
    at jodd.time.JulianDate.toLocalDateTime(JulianDate.java:368)
    at Main.main(Main.java:31)
igr commented 1 month ago

Thank you for reporting! It seems I need to re-check formulas.

So far I was checking against JPL (NASA Jet Propulsion Library), and it works like it should, meaning e.g. that year 100 is leap - except it should not be according to the Gregorian Calendar rules.

I checked two other relevant sources, and saw some differences. Will check it.

igr commented 1 month ago

The ISO 8601 uses the Gregorian calendar may not give valid answers for dates before the Gregorian calendar was adopted (15 October 1582).

Still researching :)

igr commented 1 month ago

In other words, we are returning the Julian Date and need to convert it to GregorianDate

igr commented 1 month ago

@zabetak one question - do you use other Jodd utils from the jodd-util library?

I am asking this because I would like to extract JulianDate in a separate library. Everything will stay compatible (packages, names, library).

I have noticed many uses of this class and would like to support it as much as possible, hence it might make sense to be extracted, properly explained etc.

zabetak commented 1 month ago

Since LocalDateTime adheres to the ISO-8601 the JulianDate#toLocalDateTime method should be valid conversion from the Julian calendar to the ISO calendar so "a fix for LocalDateTime, using the rules for Proleptic Gregorian Calendar" seems a reasonable option.

Regarding the custom date/time class maybe it would make sense to make the JulianDate implement the ChronoLocalDateTime interface instead of introducing new classes for the same purpose.

For the usage of jodd-utils in Apache Hive here is what I found:

grep -R "^import.*jodd.*" | sed 's/.*import//' | sort -u
 jodd.exception.UncheckedException;
 jodd.net.HtmlEncoder;
 jodd.time.JulianDate;
 static jodd.util.ClassUtil.isAssignableFrom;

Even if JulianDate is extracted in a separate library that should be fine with us.

igr commented 1 month ago

Cool, thanx!

I will consider the ChronoLocalDateTime - basically, all I need is to store date and time values.

igr commented 1 month ago

@zabetak I fixed the issue, but I had to rewrite the class. It will be released as a separate library (as I want to maintain it), with significant amount of documentation.

Currently, I am adding some more test-cases.

Sorry for taking me this much, but I had to double-check formulas. It will be done and released during this week.

zabetak commented 1 month ago

Cool, many thanks for working on this! Once you have it ready, let me know I can test the new class with Hive to see what it gives.

simhadri-g commented 1 month ago

Thank you @igr and @zabetak for this fix :)

igr commented 1 month ago

@zabetak please take a look here: https://github.com/igr/julian-day

Complete new repo, new artifact.

The changes on your side should be minimal. What was:

JulianDate.of(....)

now is

JulianDay.ofGregorianDate(....)

Of course, you may use a Julian calendar instead of the Gregorian one if it fits better.

Please let me know how this works for you and if you need any further help.