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

missing `date` function #373

Closed
joethorley opened this Issue Jan 21, 2016 · 8 comments

Comments

Projects
None yet
3 participants
@joethorley
Contributor

joethorley commented Jan 21, 2016

lubridate provides day, hour and year functions to get and set the day, hour and year components of a date time. A logical addition would be a date function to get and set the date component. It would circumvent the use of as.Date(,tz) which can cause problems if tz is set wrong. It would conflict with the base version of date which is poorly named and effectively replaced by lubridate::today. It could be called d8 to avoid any conflicts.

Please let me know if I am missing something.

I am willing to implement.

@vspinu

This comment has been minimized.

Member

vspinu commented Jan 22, 2016

Closely related to #355.

I would call it date for the sake of consistency. Users of lubridate are very unlikely to use base::date anyways.

@hadley, @huftis, what do you think?

@hadley

This comment has been minimized.

Member

hadley commented Jan 22, 2016

Hmmm, I'm not a big fan of overriding base R functions (although I agree it wouldn't make much difference here). Why not call it make_date() to be consistent with make_datetime()?

@vspinu

This comment has been minimized.

Member

vspinu commented Jan 22, 2016

Why not call it make_date() to be consistent with make_datetime()?

make_datetime is a constructor. The proposal is to add a "getter" and "setter" for date. That is, to extend the existent family of our accessors -year, day, month etc.

@joethorley

This comment has been minimized.

Contributor

joethorley commented Jan 22, 2016

get_date and set_date could work?

@hadley

This comment has been minimized.

Member

hadley commented Jan 22, 2016

Ooooh, I totally misread that

@joethorley

This comment has been minimized.

Contributor

joethorley commented Jan 23, 2016

Unless someone tells me otherwise I'm going to do a pull request early next week for get_date and set_date

@vspinu

This comment has been minimized.

Member

vspinu commented Jan 23, 2016

get_date and set_date is pretty bad IMO. set_date conflicts with R's foo<- convention and makes our accessors heterogeneous, which I don't like at all.

Overwriting date is a small price to pay for clarity and consistency of date(x) and date(x) <- d. Let's just go with overwrite for now and change it in a later stage if we happen to have a better idea.

@joethorley

This comment has been minimized.

Contributor

joethorley commented Jan 23, 2016

Agreed - I'll implement early next week

Sent from my iPhone

On Jan 22, 2016, at 22:02, Vitalie Spinu notifications@github.com wrote:

get_date and set_date is pretty bad IMO. It conflicts with R's foo<-
convention and makes our accessors heterogeneous, which I don't like at
all.

Overwriting date is a small price to pay for clarity and consistency of
date(x) and date(x) <- d. Let's just go with overwrite for now and change
it in a later stage if we happen to have a better idea.


Reply to this email directly or view it on GitHub
#373 (comment).

@vspinu vspinu closed this in 169d2ff Feb 3, 2016

vspinu added a commit that referenced this issue Feb 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment