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

[Python][R] Temporal floor/ceil/round for should accept frequency string #30745

Open
asfimport opened this issue Jan 4, 2022 · 6 comments
Open

Comments

@asfimport
Copy link

Follow up to ARROW-14822. More user-friendly rounding period input can be supported, e.g. 1D6H. See Pandas to_offset and lubridate's period.

Reporter: Rok Mihevc / @rok
Watchers: Rok Mihevc / @rok

Note: This issue was originally created as ARROW-15250. Please see the migration documentation for further details.

@asfimport
Copy link
Author

Joris Van den Bossche / @jorisvandenbossche:
I think there was also still some doubt on the PR about whether this is actually needed. The question is also how far we would do (eg lubridate seems to support both ISO8601 durations (https://en.wikipedia.org/wiki/ISO_8601#Durations) as more custom abbreviations).

@asfimport
Copy link
Author

Rok Mihevc / @rok:
The one benefit from shorthands I see is if you want to do an interval of "1D1s" that's cleaner to write as unit="second", multiple=86401 (==24h*60min*60s + 1s).
I'm happy to mark this as Not A Problem and revisit later if users want it.

@asfimport
Copy link
Author

Joris Van den Bossche / @jorisvandenbossche:
On the other hand, rounding by "1 day and 1 second" is also quite a rare use case, I think? In most common cases, the potential calculation into a multiple and unit will be quite straightforward.

@asfimport
Copy link
Author

Rok Mihevc / @rok:
Another benefit of having this is to be able to seamlessly replace lubridate/pandas.
I would prefer not to include this feature, just being completist.

@asfimport
Copy link
Author

Dewey Dunnington / @paleolimbot:
I don't think this will be an issue from a lubridate completist standpoint, since it's hard to round to a period like the one mentioned here:

lubridate::round_date(Sys.time(), unit = "1M1S")
#> Error in parse_period_unit(unit): Cannot't parse heterogenuous or fractional units larger than one minute.

@asfimport
Copy link
Author

Rok Mihevc / @rok:
But that's kind of arbitrary, see: tidyverse/lubridate#661

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

No branches or pull requests

1 participant