Skip to content
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

fixed default fractional julian day number calculation #40

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

micampe
Copy link
Contributor

@micampe micampe commented Nov 5, 2024

This fixes the Julian day calculation I mentioned in #38.

Not entirely sure on the naming I chose, let me know what you think.

Formula deduced from information at https://aa.usno.navy.mil/data/JulianDate

@helje5
Copy link
Member

helje5 commented Nov 5, 2024

Hm, I think my rational was that one would store a TimeInterval into such. I can see that SQLite uses the Julian day here https://www.sqlite.org/quirks.html#no_separate_datetime_datatype

I'm a bit split on this.

@micampe
Copy link
Contributor Author

micampe commented Nov 5, 2024

I have no stake in this, I am not using it, I fixed it just because I noticed the discrepancy.

I agree, unclear which choice is the best.

a couple of notes:

  • TimeInterval makes sense given the target of this library
  • might be confusing for someone using sqlite's date time functions and getting incorrect results
  • the default is julian number but can be overridden

@helje5
Copy link
Member

helje5 commented Nov 5, 2024

Funny enough SQLite always inserts a string for current_timestamp:

CREATE TABLE test ( 
  x TEXT, 
  tt TEXT NOT NULL DEFAULT current_timestamp, 
  rr REAL NOT NULL DEFAULT current_timestamp, 
  ii INTEGER NOT NULL DEFAULT current_timestamp
);
INSERT INTO test(x) VALUES ( 'Hello' );
SELECT * FROM test;
Hello|2024-11-05 17:50:26|2024-11-05 17:50:26|2024-11-05 17:50:26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants