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

datetime creation methods fill in "current" timezone and offset (of the person running the current JVM) #129

Open
jurgenvinju opened this issue Sep 27, 2021 · 6 comments

Comments

@jurgenvinju
Copy link
Member

That behavior (see subject) kills the possibility of treating datetime values as immutable facts. If the information about the timezone and the offset is unknown it should remain unknown. Now this default behavior is (literally) subjective and it is added implicitly without the user's explicit consent.

Will change this to using zulu time in case the information is missing, for lack of explicitly marking it is missing.

@DavyLandman
Copy link
Member

Just want to write down that this is a much larger issue, that will entail some significant rework on how we work with datetimes in vallang.

But want to leave an important point, changing to Zulu time is almost never what the user intended. Using current time zone is at least a reasonable option, it might be wrong, but it has a reasonable chance of being right. if we always convert to Zulu timezone, both the returned literal will be different, and except if you are living at offset 0:00, not what you wanted.

@jurgenvinju
Copy link
Member Author

I agree. Both behaviors are utterly wrong. The current behavior is however more insipid than changing to Zulu time IMHO, because the same code executed by two different people in different timezones will result in different results, and even the same code executed at different times in the same timezone might result in different results. That is not what any data/code scientist needs. The result of their code must be reproducible.

I don't even know if choosing Z time would do that BTW. We have to look at the Draft RAP and reconsider basically everthing of the design of datetimes in both vallang and Rascal.

@DavyLandman
Copy link
Member

DavyLandman commented Sep 28, 2021

I mean, that if I write

rascal> $2020-01-01T00:00:00$

and get back the following value:

$2019-31-12T23:00:00Z$

that is a lot more confusing than getting back:

$2020-01-01T00:00:00+01:00$

I agree it's confusing and less consistent across runs on different machines, but moving to zulu time will just make it consistently wrong.

Moving to a model where we have multiple versions of date times (with and without offsets) I agree is the better place. But I only wanted to leave a comment that we shouldn't rush this issue as a temporary fix, since it only makes it wrong in a different way. The draft RAP is very drafty, so I agree, we should redesign date time completly.

@jurgenvinju
Copy link
Member Author

Thanks! Yes, that is equally confusing.

@jurgenvinju
Copy link
Member Author

I really don't like the state that we have left this in; but it is what it is. Let's put this issue on hold and let it do its damage until we fix the design.

@jurgenvinju
Copy link
Member Author

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

No branches or pull requests

2 participants