Add calendar as a 2nd argument to Calendar.strftime/3 callbacks

[Kip]

Proposal summary

Change the callback function signature for `Calendar.strftime/3` options to pass both a number and the first arguments calendar.

Explanation

Today `Calendar.strftime/3` allows a function to be provided as an option to any of the keywords. This is very helpful when working with localised calendars because a simple function call can return all the data relevant to the localised date.  For example:

iex> Calendar.strftime ~D[2020-01-30 Cldr.Calendar.US], "%a", MyApp.Cldr.Calendar.strftime_options!()

“Fri"

But there’s an issue:  That date is actually a Thursday, not Friday!  So what’s going on?  Internally `Calendar.strftime/3` is calling `Date.day_of_week/1`. That function returns a number representing the nth day of the calendar-relative week (so-called ordinal day).  There is the same issue with month of year because not all calendars start in January. For example, the Australia tax year calendar starts in July. 

Since the US official calendar starts its weeks on Sunday - unlike Calendar.ISO which starts its weeks on Monday (like many territories) - the day name lookup failed to show the correct result. This is because the callback didn’t have the calendar context to know what the default start-of-week day is.

Now that can be worked around by specifying the calendar in MyApp.Cldr.Calendar.strftime_options!/1 like this:

iex> Calendar.strftime ~D[2020-01-30 Cldr.Calendar.US], "%a", MyApp.Cldr.Calendar.strftime_options!(calendar: Cldr.Calendar.US)

“Thu"

But that feels an error prone. Perhaps it would be better to pass the calendar of the date/datetime/time argument to the callback functions instead?  Today those functions receive a number only. This proposal is to have the functions receive both a number and the calendar of the first argument (which might be nil). This would give appropriate context to the 4 out of the 5 callback functions that would benefit from it (2 each for month name and day names).

Precedent

This would be a breaking change for the callback signature - but not for the Calendar.strftime/3 public function.  There is some limited precendent. For example when the `Calendar.day_of_week/3` callback changed to be `Calendar.day_of_week/4`. That change affected the implementation of alternative calendars, but it did not affect consumers of those calendars.

[Kip]

Hmm, I suppose the flaw in my argument is that the optional argument functions aren't really callbacks, and they are part of the public API. Which means this is a breaking change for anyone using a function value for an option.

[José Valim]

I can think of a few options:

1. Make day_of_week return a cardinal value in this case by passing :monday as argument to Date.day_of_week

2. Same as above but make the starting day_of_week an option, so you could return starting_day_of_week as part of your options (and you will probably default to :monday on all of them anyway)

However, I believe the issue above is not specific to day_of_week? If it is not:

3. Change all callbacks functions in strftime to optionally receive a calendar. We could check the function arity and dispatch accordingly

4. Same as above but you do it in CLDR.strftime, which you add, and wrap all functions to receive the Calendar

My preference, for simplicity, would be 1 or 2, probably 2 for backwards compatibility.

José Valim

https://dashbit.co/

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/elixir-lang-core/583251f8-b182-4133-8a9d-03bc3b96ae74n%40googlegroups.com.

[Kip]

> However, I believe the issue above is not specific to day_of_week?

Yes, the same issue with month of year which I mentioned briefly above. So I don't believe option (1) and (2) would work for month of year.

Option (3) would be my preference, but arity-checking isn't beautiful I agree. 

Option (4) isn't ideally in scope for ex_cldr since Cldr.DateTime.to_string/3 is the canonical CLDR standard formatter.

If (1) and (2) won't work (because of the same issue for both day of week and month of year) and (3) and (4) aren't considered acceptablethen:

(a) I'll submit a PR to update the documentation for strftime/3 to note the behaviour when the calendars first day of week isn't Monday

(b) I'll Document in ex_cldr_calendars the importance of passing in a calendar to `MyApp.Cldr.Calendar.strftime_options!/1` for localised calendars

[José Valim]

> Yes, the same issue with month of year which I mentioned briefly above.

Do we have the same issue with "month of year" in that it is representing the cardinal month instead of the ordinal one?

> Option (4) isn't ideally in scope for ex_cldr since Cldr.DateTime.to_string/3 is the canonical CLDR standard formatter.

WDYM? If that's the standard formatter, does it call Calendar.strftime behind the scenes? Regardless, could you add Cldr.DateTime.strftime or would that violate some CLDR API rule/principles?

---

Option 3 is something we could accept in Elixir but option 4 means you can deliver the feature today, rather than in ~4 months. :)

José Valim

https://dashbit.co/

To view this discussion visit https://groups.google.com/d/msgid/elixir-lang-core/be73d5e9-2451-48c9-a718-5a961eebfc48n%40googlegroups.com.

[Kip]

José, thank you. I have implemented Cldr.Calendar.strftime/2 as you suggested and published it as part of ex_cldr_calendars 2.0.  With your agreement I'll submit a PR for option (3) for core team consideration.

[José Valim]

Yes, let’s do option 3 as well. And now that I think more about it, we should just pass whatever is given to strftime as first argument to the anonymous function depending on the arity. That should cover all bases.

José Valim

https://dashbit.co/

To view this discussion visit https://groups.google.com/d/msgid/elixir-lang-core/154e46b6-b7c7-4f64-8e4a-c41140767bcfn%40googlegroups.com.