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

Consider renaming NormalizeDuration and UnnormalizeDuration #2953

Closed
ptomato opened this issue Sep 24, 2024 · 4 comments · Fixed by #3007
Closed

Consider renaming NormalizeDuration and UnnormalizeDuration #2953

ptomato opened this issue Sep 24, 2024 · 4 comments · Fixed by #3007
Labels
editorial non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Milestone

Comments

@ptomato
Copy link
Collaborator

ptomato commented Sep 24, 2024

It'd be helpful to include an explanatory note somewhere in the spec (if not there already) explaining the three types of durations (duration records?) in your comment, which is very clear!

(the referenced comment is this:)

"Normalized" duration is the form in which implementations prefer to perform calculations. It consists of 32 bits of years, 32 bits of months, 32 bits of weeks, 32 bits of days if not assuming 24-hour days, and all the other fields squished into 74 bits of nanoseconds.

"Unnormalized" duration is the form in which a duration is presented to users, consisting of 10× 64-bit floating point fields.

Unnormalized durations may be "balanced" up to a certain unit, or may not be. I suppose normalized durations are by definition not balanced.

re: naming, after thinking about the concepts involved, I wonder if these names would be clearer?

  • Internal Duration Record - this is what's currently called normalized
  • User-Facing Duration Record OR Observable Duration Record OR Public Duration Record - this is what's currently called unnormalized.

Ironically, "normalized" might actually be a better name for our existing concept of "balanced", but it's probably too late to change that.

Originally posted by @justingrant in #2944 (comment)

@ptomato ptomato added spec-text Specification text involved non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! editorial labels Sep 24, 2024
@ptomato
Copy link
Collaborator Author

ptomato commented Oct 3, 2024

Also consider renaming UnbalanceDateDurationRelative, which may also be confused with "balanced" durations.

@justingrant
Copy link
Collaborator

justingrant commented Oct 3, 2024

Meeting 2024-10-03: Rename the following

  • Normalized Duration Record => Internal Duration Record
  • NormalizeDuration => ToInternalDurationRecord
  • Unnormalized Duration Record => Public Duration Record
  • UnnormalizeDuration => ToPublicDurationRecord
  • UnbalanceDateDurationRelative => DateDurationDays
  • Normalized Time Duration Record => time duration (a 2-word definition that constrains the range of an integral mathematical value, not a record)
  • *NormalizedTimeDuration* => *TimeDuration* (many AOs)

@ptomato ptomato added this to the Stage "3.5" milestone Oct 3, 2024
@ptomato
Copy link
Collaborator Author

ptomato commented Oct 8, 2024

It completely escaped me when we discussed this, that non-normalized Duration Records are no longer a thing since #2943. We just use Temporal.Duration in that case. So no need for Public Duration Record. Instead of ToPublicDurationRecord, I propose renaming UnnormalizeDuration to TemporalDurationFromInternal. Sound good?

@justingrant
Copy link
Collaborator

Sounds good!

ptomato added a commit that referenced this issue Oct 8, 2024
"Normalized" was jargon that we adopted after the February 2023 TC39
meeting. "Internal" is clearer; we convert a Temporal.Duration into an
"internal" record for calculations.

See: #2953
ptomato added a commit that referenced this issue Oct 8, 2024
This operation used to do more than just calculate the number of days, and
the name used to make more sense when we had BalanceRelative, etc. Now
it's just confusing. Rename it to reflect what it does.

See: #2953
ptomato added a commit that referenced this issue Oct 8, 2024
A normalized time duration is just a number of nanoseconds; we originally
planned to convert it to a record with a seconds and subseconds field, but
that was never needed. So we don't need to have it be a record in the
first place. Rename it to "time duration" (removing "normalized" for the
same reason as in the previous commits) and define it as an integer within
a certain range.

Renames:
- all of the NormalizedTimeDuration___ operations to just TimeDuration___
- NormalizeTimeDuration to TimeDurationFromComponents
- Internal duration [[NormalizedTime]] field to [[Time]]
- "norm" variables to "timeDuration" or "time" if clear from context

See: #2953
ptomato added a commit that referenced this issue Oct 8, 2024
We originally planned to convert time durations to a record with a seconds
and subseconds field, but that was never necessary. Now that they are just
a mathematical value, we don't need to access them through these seconds
and subseconds operations.

See: #2953
ptomato added a commit that referenced this issue Oct 8, 2024
It now just returns 0. No need to have an operation for that.

See: #2953
ptomato added a commit that referenced this issue Oct 9, 2024
"Normalized" was jargon that we adopted after the February 2023 TC39
meeting. "Internal" is clearer; we convert a Temporal.Duration into an
"internal" record for calculations.

See: #2953
ptomato added a commit that referenced this issue Oct 9, 2024
This operation used to do more than just calculate the number of days, and
the name used to make more sense when we had BalanceRelative, etc. Now
it's just confusing. Rename it to reflect what it does.

See: #2953
ptomato added a commit that referenced this issue Oct 9, 2024
A normalized time duration is just a number of nanoseconds; we originally
planned to convert it to a record with a seconds and subseconds field, but
that was never needed. So we don't need to have it be a record in the
first place. Rename it to "time duration" (removing "normalized" for the
same reason as in the previous commits) and define it as an integer within
a certain range.

Renames:
- all of the NormalizedTimeDuration___ operations to just TimeDuration___
- NormalizeTimeDuration to TimeDurationFromComponents
- Internal duration [[NormalizedTime]] field to [[Time]]
- "norm" variables to "timeDuration" or "time" if clear from context

See: #2953
ptomato added a commit that referenced this issue Oct 9, 2024
We originally planned to convert time durations to a record with a seconds
and subseconds field, but that was never necessary. Now that they are just
a mathematical value, we don't need to access them through these seconds
and subseconds operations.

See: #2953
ptomato added a commit that referenced this issue Oct 9, 2024
It now just returns 0. No need to have an operation for that.

See: #2953
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants