Re: [elixir-core:11751] [Proposal] Add `shift/2-3` to calendar types

2024-05-08 Thread José Valim
Hi Kip, please send a PR. I think this will be easier to see in code but
what you said makes sense on paper. :)

On Thu, May 9, 2024 at 02:19 Kip  wrote:

> I'm just now implementing the new callbacks for `ex_cldr_calendars` and in
> reviewing the implementation for `Calendar.ISO` is strikes me that the
> whole implementation, except for one line (see below) depends only on other
> calendar callbacks. And therefore could be moved into the `Calendar` module
> and used as an implementation for any calendar that implements `Calendar`
> behaviour.
>
> This line
> 
>  would
> need to change to use `calendar.months_in_year/1` and a few other calls
> would need to change to be calendar-referenced to. Moving the code might
> also allow centralising the options handling and exceptions - it's a little
> unusual for me to see callbacks handling the options validation rather than
> the public API.
>
> I am more than happy to submit a PR for this very small refactor (thanks
> to the very clean implementation) if this idea is considered to have merit.
>
>
> On Saturday, March 9, 2024 at 1:42:34 AM UTC+11 br...@grox.io wrote:
>
>> The biggest question is if we consider the fields in Duration a unit or
>> not. If they are units, then the most consistent choice is to keep them
>> singular, to mirror System.time_unit and friends.
>>
>>
>> This is the API I prefer: units. IMHO, it is more important to keep
>> consistency with Elixir libraries.
>>
>> -bt
>>
>> On Thu, Mar 7, 2024 at 11:02 PM José Valim  wrote:
>>
>>> It is worth noting that Date and friends in Elixir require a calendar
>>> field, which is not present in Duration, and therefore Duration won't be
>>> usable as Date (and friends).
>>>
>>> The biggest question is if we consider the fields in Duration a unit or
>>> not. If they are units, then the most consistent choice is to keep them
>>> singular, to mirror System.time_unit and friends.
>>>
>>> On Fri, Mar 8, 2024 at 4:55 AM Kip Cole  wrote:
>>>
 In my head, a Date.t is semantically a duration. So it’s completely
 valid to pass it as a duration to Date.shift as I see it. Which argues for
 singular names.

 This conversation is a bit like “is a date a point in time or an
 interval”. And the answer is yes, depending.

 Sent from my iPhone

 On 8 Mar 2024, at 14:39, Sabiwara Yukichi  wrote:

 
 I'm personally leaning more towards the plural, because it feels
 semantically more correct to treat a point of time and a duration as
 separate.

 d.year means the same thing if d is either a date or a datetime, but
 for a duration calling it d.years emphasizes the difference.

 It could perhaps help catch errors as well, both for the human and the
 compiler.
 One (arguably contrived) example would be structurally typed code which
 doesn't enforce any type in particular but uses the dot access or partial
 pattern matches like %{year: ..., month: ...} in order to support both
 dates or datetimes. Passing in a duration wouldn't make sense semantically,
 having different names would make it fail properly.

 I also agree with other reasons mentioned, the known standard one
 especially.

 Le jeu. 7 mars 2024 à 16:07, 'Theo Fiedler' via elixir-lang-core <
 elixir-l...@googlegroups.com> a écrit :

> Right, it would make using a Duration in combination with the
> `add/2-3` functions much harder than it needs to be. So far all time units
> in Elixir are singular, and I think we do gain something from consistently
> sticking to that, regardless of the context of durations, calendar types
> and what not.
>
> I've seen some libraries allowing both, singular and plural, which i
> dont want to have anything to do with, except for mentioning it though 
> haha.
>
> What i currently see is:
>
> Reasons for plural:
> - Known standard across various libraries and programming languages
> - Sounds natural, to shift a date by "3 months" instead of "3 month".
>
> Reasons for singular:
> - Compatible with time units already defined in Elixir (also relevant
> for extending the use of Duration later on)
> - Reduced cognitive load as the time units are always spelled the same
> regardless of the context
>
> The reasons for singular do outweigh the reasons for plural, so unless
> we're making some very strong points for diverging from that, let's keep 
> it
> singular!
>
> On Thursday, March 7, 2024 at 7:39:15 AM UTC+1 José Valim wrote:
>
>> Compatibility with the other time units is an important point. My
>> mind is back on singular again. :)
>>
>> On Thu, Mar 7, 2024 at 07:20 'Theo Fiedler' via elixir-lang-core <
>> elixir-l...@googlegroups.com> wrote:
>>
>>> While i was strongly leaning 

Re: [elixir-core:11751] [Proposal] Add `shift/2-3` to calendar types

2024-05-08 Thread Kip
I'm just now implementing the new callbacks for `ex_cldr_calendars` and in 
reviewing the implementation for `Calendar.ISO` is strikes me that the 
whole implementation, except for one line (see below) depends only on other 
calendar callbacks. And therefore could be moved into the `Calendar` module 
and used as an implementation for any calendar that implements `Calendar` 
behaviour.

This line 

 would 
need to change to use `calendar.months_in_year/1` and a few other calls 
would need to change to be calendar-referenced to. Moving the code might 
also allow centralising the options handling and exceptions - it's a little 
unusual for me to see callbacks handling the options validation rather than 
the public API.

I am more than happy to submit a PR for this very small refactor (thanks to 
the very clean implementation) if this idea is considered to have merit.


On Saturday, March 9, 2024 at 1:42:34 AM UTC+11 br...@grox.io wrote:

> The biggest question is if we consider the fields in Duration a unit or 
> not. If they are units, then the most consistent choice is to keep them 
> singular, to mirror System.time_unit and friends.
>
>
> This is the API I prefer: units. IMHO, it is more important to keep 
> consistency with Elixir libraries. 
>
> -bt
>
> On Thu, Mar 7, 2024 at 11:02 PM José Valim  wrote:
>
>> It is worth noting that Date and friends in Elixir require a calendar 
>> field, which is not present in Duration, and therefore Duration won't be 
>> usable as Date (and friends).
>>
>> The biggest question is if we consider the fields in Duration a unit or 
>> not. If they are units, then the most consistent choice is to keep them 
>> singular, to mirror System.time_unit and friends.
>>
>> On Fri, Mar 8, 2024 at 4:55 AM Kip Cole  wrote:
>>
>>> In my head, a Date.t is semantically a duration. So it’s completely 
>>> valid to pass it as a duration to Date.shift as I see it. Which argues for 
>>> singular names.
>>>
>>> This conversation is a bit like “is a date a point in time or an 
>>> interval”. And the answer is yes, depending.
>>>
>>> Sent from my iPhone
>>>
>>> On 8 Mar 2024, at 14:39, Sabiwara Yukichi  wrote:
>>>
>>> 
>>> I'm personally leaning more towards the plural, because it feels 
>>> semantically more correct to treat a point of time and a duration as 
>>> separate.
>>>
>>> d.year means the same thing if d is either a date or a datetime, but for 
>>> a duration calling it d.years emphasizes the difference.
>>>
>>> It could perhaps help catch errors as well, both for the human and the 
>>> compiler.
>>> One (arguably contrived) example would be structurally typed code which 
>>> doesn't enforce any type in particular but uses the dot access or partial 
>>> pattern matches like %{year: ..., month: ...} in order to support both 
>>> dates or datetimes. Passing in a duration wouldn't make sense semantically, 
>>> having different names would make it fail properly.
>>>
>>> I also agree with other reasons mentioned, the known standard one 
>>> especially.
>>>
>>> Le jeu. 7 mars 2024 à 16:07, 'Theo Fiedler' via elixir-lang-core <
>>> elixir-l...@googlegroups.com> a écrit :
>>>
 Right, it would make using a Duration in combination with the `add/2-3` 
 functions much harder than it needs to be. So far all time units in Elixir 
 are singular, and I think we do gain something from consistently sticking 
 to that, regardless of the context of durations, calendar types and what 
 not.

 I've seen some libraries allowing both, singular and plural, which i 
 dont want to have anything to do with, except for mentioning it though 
 haha.

 What i currently see is:

 Reasons for plural:
 - Known standard across various libraries and programming languages
 - Sounds natural, to shift a date by "3 months" instead of "3 month".

 Reasons for singular:
 - Compatible with time units already defined in Elixir (also relevant 
 for extending the use of Duration later on)
 - Reduced cognitive load as the time units are always spelled the same 
 regardless of the context

 The reasons for singular do outweigh the reasons for plural, so unless 
 we're making some very strong points for diverging from that, let's keep 
 it 
 singular!

 On Thursday, March 7, 2024 at 7:39:15 AM UTC+1 José Valim wrote:

> Compatibility with the other time units is an important point. My mind 
> is back on singular again. :)
>
> On Thu, Mar 7, 2024 at 07:20 'Theo Fiedler' via elixir-lang-core <
> elixir-l...@googlegroups.com> wrote:
>
>> While i was strongly leaning towards singular, i understand why one 
>> would expect plural. Given that seems to be pretty standard in wild, i 
>> am 
>> fine changing it as well.
>>
>> What mostly put me off about was that we'd end up with `Time.add(t,