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

Basic time types #1

Merged
merged 21 commits into from
Dec 30, 2024
Merged

Basic time types #1

merged 21 commits into from
Dec 30, 2024

Conversation

lpil
Copy link
Member

@lpil lpil commented Dec 23, 2024

Discussion: gleam-lang/gleam#4095

How does this design look?

Are there any other functions that we would want to have initially?

If we are happy with the overall design I could merge this and we could make PRs for the functions and documentation.

@jrstrunk
Copy link

I think functions to create a timestamp from milliseconds and microseconds would be nice too! Millisecond (and microsecond to a lesser degree) int values will be common when interacting with external sources!

@lpil
Copy link
Member Author

lpil commented Dec 23, 2024

Could you give some examples of those millisecond and microsecond Unix time sources @jrstrunk 🙏

@jrstrunk
Copy link

Consuming external APIs is mainly what I was thinking of. This was the first to come to mind, but it is a little overkill with nanosecond timestamps: https://polygon.io/docs/stocks/get_v2_last_nbbo__stocksticker. I have seen it many times in proprietary APIs that I don't have access to anymore. Here is another one that gives the timestamp as a fractional second: https://platform.openai.com/docs/api-reference/audio/verbose-json-object. Again not exactly needing milliseconds or microseconds but they do exist!

@lpil
Copy link
Member Author

lpil commented Dec 23, 2024

My hunch is that the existing from-nanoseconds will be enough given that nano and milli and micro are rather rare, so asking people to convert is OK.

@GearsDatapacks
Copy link
Member

GearsDatapacks commented Dec 23, 2024

Could that run into the max unsafe integer on javascript though?
Edit: just read the code, nevermind

@jrstrunk
Copy link

jrstrunk commented Dec 23, 2024

That's fine with me! Anecdotally I seem to think millisecond timestamps are more common than second timestamps, but time and user feedback will tell, no need to worry about it too much now :) and it's not like the conversion is hard haha

@lpil lpil marked this pull request as ready for review December 27, 2024 14:03
Copy link
Member

@GearsDatapacks GearsDatapacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I'm not familiar with the string formats though, so I can't verify that those are correct

src/gleam/time/duration.gleam Outdated Show resolved Hide resolved
src/gleam/time/duration.gleam Show resolved Hide resolved
src/gleam/time/duration.gleam Show resolved Hide resolved
src/gleam/time/duration.gleam Show resolved Hide resolved
src/gleam/time/duration.gleam Outdated Show resolved Hide resolved
src/gleam/time/timestamp.gleam Show resolved Hide resolved
src/gleam/time/timestamp.gleam Show resolved Hide resolved
export function system_time() {
const now = Date.now();
const milliseconds = now % 1_000;
const nanoseconds = milliseconds * 1000_000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this stopping at nanoseconds lik the Erlang equivalent?

Copy link
Member Author

@lpil lpil Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, not sure what stopping at nanoseconds means.

If you're asking why it doesn't have nanosecond precision it's because JS doesn't have an API for system time with nanosecond precision

src/gleam/time/duration.gleam Outdated Show resolved Hide resolved
let day_of_year =
day_of_era
- { 365 * year_of_era + { year_of_era / 4 } - { year_of_era / 100 } }
let mp = { 5 * day_of_year + 2 } / 153
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is mp?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly cannot remember. Might be that I got it from the reference and wasn't sure what it meant there either

@lpil lpil merged commit 5d3abb4 into main Dec 30, 2024
1 check passed
@lpil lpil deleted the time branch December 30, 2024 21:07
@jrstrunk jrstrunk mentioned this pull request Dec 30, 2024
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.

5 participants