Feature Request: Support for "week" in floor_date function

[mistermichaelll]

Hello! Love the Tidier project, and have just started trying to use it in my own workflows!

One thing that I find myself doing often in R is to take a set of dates and aggregate it on a weekly basis, with the week starting on Sunday. Using lubridate, that typically looks something like this:

df |>
     mutate(week = lubridate::floor_date(dt, "week") |> 
     group_by(week) |> 
     summarize(
          n_entries = n(),
          total_sales = sum(sales)
     )

However, "week" is not a supported argument in floor_date in TidierDates.jl. I did some initial exploration of this, and noticed that Dates does have a firstdayofweek function which returns the first Monday of a week given some TimeType. lubridate by default uses Sunday as the start of the week, so my first pass at a solution to add this functionality to TidierDates.jl looks like this:

function floor_date(dt::Union{DateTime, Missing}, unit::String)
    if ismissing(dt)
        return missing
    end
# ...
    elseif unit == "week"
        start_of_week = firstdayofweek(dt) - Day(1)
        return DateTime(year(start_of_week), month(start_of_week), day(start_of_week))
# ...
end

Which uses Dates.firstdayofweek to determine the Monday of the week of a provided date, then subtracts a day to return the Sunday (again, the base behavior of lubridate in R if you don't override the week_start option).

Now of course, one could do something like this without floor_date. But it would be very convenient for "week" to be supported in this function!

I have a few questions on this:

1. is there a reason that "week" was not supported initially?
2. I realize that this may not be the most elegant solution 😅 are there changes I can make to the solution above that would be accepted as a PR for adding this functionality?

[drizk1]

First off I'm so happy you like the Tidier and find it enjoyable and valuable in your work flow.

There's no reason why I didn't include week initially besides I was new to coding and trying to get a prelim version available.

I think your solution looks mighty elegant.

If you throw it in a PR and add a docstring example that passes for it in the floor_date in addition to clarifying that starts in Sunday in the docs I'd be happy to merge

[mistermichaelll]

There's no reason why I didn't include week initially besides I was new to coding and trying to get a prelim version available.

Sounds good! I just wanted to make sure the reason wasn't like "it is actually extremely complicated and will break everything if we try to add this." 😄

I'll go ahead and add the docstrings and put a PR together.

[drizk1]

Sounds good! Thank you !

[drizk1]

awesome, ill go ahead and bump version and release momentarily here