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

Add to_iso8601/1 for duration #7

Open
kipcole9 opened this issue Jun 10, 2020 · 2 comments
Open

Add to_iso8601/1 for duration #7

kipcole9 opened this issue Jun 10, 2020 · 2 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@kipcole9
Copy link
Collaborator

ISO8601 defines a duration format. Add to_iso8601 to convert Cldr.Calendar.Duration.t to this format. And from_iso8601/1 to do the reverse.

For now a Cldr.Calendar.Duration/1 does not support:

  1. Week durations
  2. Negative durations
  3. Durations where the date/time components are unordered
@kipcole9 kipcole9 added the enhancement New feature or request label Jun 10, 2020
@kipcole9 kipcole9 added this to the 1.10.0 milestone Jun 10, 2020
@kipcole9 kipcole9 self-assigned this Jun 10, 2020
@tanguilp
Copy link

Duration.from_iso8601/1 and Duration.to_iso8601/1 were implemented in Elixir 1.17. Implementing it is no longer needed it seems,

How do you think we could update Cldr.Calendar.Duration to make use of the new Duration module? Types have some differences:

  • Elixir's Duration.t accepts negative values
  • Elixir's Duration.t has a week component
  • Elixir's Duration.t microsecond has a precision component

Some ideas:

  • accept both Cldr.Calendar.Duration.t and Duration.t as parameters of to_string/2
  • add a conversion function Cldr.Calendar.Duration.from_duration

In both case, we need to handle negative duration, by rejecting them for instance.

@kipcole9
Copy link
Collaborator Author

Thank you for the thoughtful ideas @tanguilp. My current thinking is the prioritise implementing the new Elixir 1.17 Calendar callbacks. This requires more work that it might initially seem given that years may have a variable number of months and months have a variable number of days (you know this already of course).

I aim to have an initial version implemented this weekend for some exposure and testing.

Then I plan to deprecate the Cldr Durations but keep supporting them until Elixir 1.17 is the minimum supported version - that will be quite some time since I’m only now making Elixir 1.12 the minimum version. Perhaps I should be more aggressive in moving up the minimum support line.

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

No branches or pull requests

2 participants