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

Fractional parts should not be limited to nine digits #1712

Closed
gibson042 opened this issue Aug 10, 2021 · 8 comments
Closed

Fractional parts should not be limited to nine digits #1712

gibson042 opened this issue Aug 10, 2021 · 8 comments
Labels
behavior Relating to behavior defined in the proposal iso8601-grammar ISO 8061 grammar meeting-agenda normative Would be a normative change to the proposal spec-text Specification text involved

Comments

@gibson042
Copy link
Collaborator

"PT0.0000000001H" represents exactly 360 nanoseconds, but Temporal.Duration.from rejects it with a RangeError.

@gibson042 gibson042 added behavior Relating to behavior defined in the proposal spec-text Specification text involved labels Aug 10, 2021
@ptomato
Copy link
Collaborator

ptomato commented Aug 13, 2021

Previously we had decided to reject strings with more than 9 decimal places: #198 (comment)
Although at that point, we only allowed seconds to have decimal places, so this issue wouldn't have come up.

What do you propose to change? (e.g., Duration strings with hours or minutes fractions should be allowed to have more than 9 decimal places as long as they evaluate to any nonzero integer Duration components?) That seems quite fuzzy and/or unexpected for users, given that seconds with up to 9 decimal places will be the much more common input...

@gibson042
Copy link
Collaborator Author

I would propose allowing any fractional component in a string being deserialized to use an arbitrary number of digits and throwing a RangeError if the corresponding value cannot be represented exactly.

@ptomato
Copy link
Collaborator

ptomato commented Aug 13, 2021

That seems quite dangerous, since it could make PT0.000000002S fail depending on how the operation was written: it could work out to 2.0000000000000001 nanoseconds.

@gibson042
Copy link
Collaborator Author

"how the operation was written" is up to the specification, which should be using decimal arithmetic for such deserialization.

@gibson042
Copy link
Collaborator Author

But this is also not that difficult given the finite precision cutoff for resulting values.

@ptomato
Copy link
Collaborator

ptomato commented Aug 16, 2021

I see what you mean now. I'm concerned that this would be a Stage 3 change not arising from implementation experience, though.

(Aside from that, I'm running out of time to prepare a presentation for the agenda deadline, since there are a lot more normative changes coming from implementors for me to work through. If the champions did decide to change this, it might have to wait for the October plenary.)

@ptomato ptomato added normative Would be a normative change to the proposal meeting-agenda labels May 10, 2022
@ptomato ptomato added the iso8601-grammar ISO 8061 grammar label Aug 11, 2022
@sffc
Copy link
Collaborator

sffc commented Oct 27, 2022

Note that the maximum exact minute has 10 fractional digits, and hours 11 factional digits. Anything else is tricky to verify. So I vote to keep this as-is.

@ptomato
Copy link
Collaborator

ptomato commented Oct 27, 2022

Temporal champions meeting, 2022-10-27: For just 1 or 2 extra digits in fractional hour and fractional minute strings this doesn't seem worth the increased complexity for implementations. We'll propose this for a future version of Temporal instead: js-temporal/proposal-temporal-v2#26

@ptomato ptomato closed this as completed Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior Relating to behavior defined in the proposal iso8601-grammar ISO 8061 grammar meeting-agenda normative Would be a normative change to the proposal spec-text Specification text involved
Projects
None yet
Development

No branches or pull requests

3 participants