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

support Duration in Date.range/3 #14172

Merged
merged 11 commits into from
Jan 13, 2025
55 changes: 50 additions & 5 deletions lib/elixir/lib/calendar/date.ex
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ defmodule Date do
calendar: Calendar.calendar()
}

@type duration_unit_pair ::
josevalim marked this conversation as resolved.
Show resolved Hide resolved
{:year, integer} | {:month, integer} | {:week, integer} | {:day, integer}

@doc """
Returns a range of dates.

Expand All @@ -84,6 +87,13 @@ defmodule Date do
iex> Date.range(~D[1999-01-01], ~D[2000-01-01])
Date.range(~D[1999-01-01], ~D[2000-01-01])

A range may also be built from a `Date` and a `Duration`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A range may also be built from a `Date` and a `Duration`:
A range may also be built from a `Date` and a `Duration`
(also expressed as a keyword list of `t:duration_unit_pair/0`):

?


iex> Date.range(~D[1999-01-01], Duration.new!(year: 1))
Date.range(~D[1999-01-01], ~D[2000-01-01])
iex> Date.range(~D[1999-01-01], year: 1)
Date.range(~D[1999-01-01], ~D[2000-01-01])

josevalim marked this conversation as resolved.
Show resolved Hide resolved
A range of dates implements the `Enumerable` protocol, which means
functions in the `Enum` module can be used to work with
ranges:
Expand All @@ -100,7 +110,8 @@ defmodule Date do

"""
@doc since: "1.5.0"
@spec range(Calendar.date(), Calendar.date()) :: Date.Range.t()
@spec range(Calendar.date(), Calendar.date() | Duration.t() | [duration_unit_pair]) ::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would improve docs (now they would still show range(first, last):

Suggested change
@spec range(Calendar.date(), Calendar.date() | Duration.t() | [duration_unit_pair]) ::
@spec range(start :: Calendar.date(), end_or_duration :: Calendar.date() | Duration.t() | [duration_unit_pair]) ::

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while I like start/end, should we stick to first/last so it's:

@spec range(first :: Calendar.date(), last_or_duration :: Calendar.date() | Duration.t() | [duration_unit_pair]) ::

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2025-01-13 at 10 27 29

Screenshot 2025-01-13 at 10 30 34

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last_or_duration is ok but the "duration" part doesn't work as well with first IMO, compared to start + duration/start + end. Either way not a big deal 🙃

Date.Range.t()
def range(%{calendar: calendar} = first, %{calendar: calendar} = last) do
{first_days, _} = to_iso_days(first)
{last_days, _} = to_iso_days(last)
Expand All @@ -123,6 +134,20 @@ defmodule Date do
raise ArgumentError, "both dates must have matching calendars"
end

def range(%{calendar: calendar} = first, %Duration{} = duration) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we allow the keyword list version as well? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i found that range(~D[2020-01-01], [year: 1], 2) looks a bit unergonomic, but definitely nothing speaking against allowing it 👍

last = shift(first, duration)
range(first, last)
end

def range(%{calendar: calendar} = first, duration) when is_list(duration) do
last = shift(first, duration)
range(first, last)
end

def range(%{calendar: _, year: _, month: _, day: _}, _duration) do
raise ArgumentError, "expected a date or duration as second argument"
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josevalim do we usually do this, or just let this fall through and raise a FunctionClauseError? I think this kind of thing would be caught by types anyway


@doc """
Returns a range of dates with a step.

Expand All @@ -140,8 +165,11 @@ defmodule Date do

"""
@doc since: "1.12.0"
@spec range(Calendar.date(), Calendar.date(), step :: pos_integer | neg_integer) ::
Date.Range.t()
@spec range(
Calendar.date(),
Calendar.date() | Duration.t() | [duration_unit_pair],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same naming issue here

step :: pos_integer | neg_integer
) :: Date.Range.t()
def range(%{calendar: calendar} = first, %{calendar: calendar} = last, step)
when is_integer(step) and step != 0 do
{first_days, _} = to_iso_days(first)
Expand All @@ -159,6 +187,24 @@ defmodule Date do
"non-zero integer, got: #{inspect(first)}, #{inspect(last)}, #{step}"
end

def range(%{calendar: calendar} = first, %Duration{} = duration, step)
when is_integer(step) and step != 0 do
last = shift(first, duration)
range(first, last, step)
end

def range(%{calendar: calendar} = first, duration, step)
when is_list(duration) and is_integer(step) and step != 0 do
last = shift(first, duration)
range(first, last, step)
end

def range(%{calendar: _, year: _, month: _, day: _} = first, duration, step) do
raise ArgumentError,
"expected a date or duration as second argument and the step must be a " <>
"non-zero integer, got: #{inspect(first)}, #{inspect(duration)}, #{step}"
end

defp range(first, first_days, last, last_days, calendar, step) do
%Date.Range{
first: %Date{calendar: calendar, year: first.year, month: first.month, day: first.day},
Expand Down Expand Up @@ -795,8 +841,7 @@ defmodule Date do

"""
@doc since: "1.17.0"
@spec shift(Calendar.date(), Duration.t() | [unit_pair]) :: t
when unit_pair: {:year, integer} | {:month, integer} | {:week, integer} | {:day, integer}
@spec shift(Calendar.date(), Duration.t() | [duration_unit_pair]) :: t
def shift(%{calendar: calendar} = date, duration) do
%{year: year, month: month, day: day} = date
{year, month, day} = calendar.shift_date(year, month, day, __duration__!(duration))
Expand Down
42 changes: 42 additions & 0 deletions lib/elixir/test/elixir/calendar/date_range_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ defmodule Date.RangeTest do

@asc_range Date.range(~D[2000-01-01], ~D[2001-01-01])
@asc_range_2 Date.range(~D[2000-01-01], ~D[2001-01-01], 2)
@asc_range_duration Date.range(~D[2000-01-01], Duration.new!(year: 1))
@asc_range_duration_2 Date.range(~D[2000-01-01], Duration.new!(year: 1), 2)
@desc_range Date.range(~D[2001-01-01], ~D[2000-01-01], -1)
@desc_range_2 Date.range(~D[2001-01-01], ~D[2000-01-01], -2)
@desc_range_duration Date.range(~D[2001-01-01], Duration.new!(year: -1))
@desc_range_duration_2 Date.range(~D[2001-01-01], Duration.new!(year: -1), 2)
@empty_range Date.range(~D[2001-01-01], ~D[2000-01-01], 1)

describe "Enum.member?/2" do
Expand All @@ -20,6 +24,9 @@ defmodule Date.RangeTest do

assert Enum.member?(@asc_range_2, ~D[2000-01-03])
refute Enum.member?(@asc_range_2, ~D[2000-01-02])

assert Enum.member?(@asc_range_duration, ~D[2000-01-03])
refute Enum.member?(@asc_range_duration_2, ~D[2000-01-02])
end

test "for descending range" do
Expand All @@ -31,6 +38,9 @@ defmodule Date.RangeTest do

assert Enum.member?(@desc_range_2, ~D[2000-12-30])
refute Enum.member?(@desc_range_2, ~D[2000-12-29])

assert Enum.member?(@desc_range_duration, ~D[2000-12-30])
refute Enum.member?(@desc_range_duration_2, ~D[2000-12-29])
end

test "empty range" do
Expand Down Expand Up @@ -109,6 +119,30 @@ defmodule Date.RangeTest do
assert Enum.to_list(range) == [~D[2000-01-01], ~D[2000-01-03]]
end

test "works with durations" do
range = Date.range(~D[2000-01-01], Duration.new!(day: 1))
assert range.first == ~D[2000-01-01]
assert range.last == ~D[2000-01-02]
assert Enum.to_list(range) == [~D[2000-01-01], ~D[2000-01-02]]

range = Date.range(~D[2000-01-01], Duration.new!(day: 2), 2)
assert range.first == ~D[2000-01-01]
assert range.last == ~D[2000-01-03]
assert Enum.to_list(range) == [~D[2000-01-01], ~D[2000-01-03]]
end

test "accepts durations as keyword list" do
range = Date.range(~D[2000-01-01], day: 1)
assert range.first == ~D[2000-01-01]
assert range.last == ~D[2000-01-02]
assert Enum.to_list(range) == [~D[2000-01-01], ~D[2000-01-02]]

range = Date.range(~D[2000-01-01], [day: 2], 2)
assert range.first == ~D[2000-01-01]
assert range.last == ~D[2000-01-03]
assert Enum.to_list(range) == [~D[2000-01-01], ~D[2000-01-03]]
end

test "both dates must have matching calendars" do
first = ~D[2000-01-01]
last = Calendar.Holocene.date(12001, 1, 1)
Expand All @@ -118,6 +152,14 @@ defmodule Date.RangeTest do
end
end

test "second argument is validated" do
first = ~D[2000-01-01]

assert_raise ArgumentError, "expected a date or duration as second argument", fn ->
Date.range(first, 123)
end
end

test "accepts equal but non Calendar.ISO calendars" do
first = Calendar.Holocene.date(12000, 1, 1)
last = Calendar.Holocene.date(12001, 1, 1)
Expand Down