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

pmap strips Date #358

Closed
AkhilGNair opened this issue Aug 1, 2017 · 19 comments
Closed

pmap strips Date #358

AkhilGNair opened this issue Aug 1, 2017 · 19 comments

Comments

@AkhilGNair
Copy link

@AkhilGNair AkhilGNair commented Aug 1, 2017

As title - purrr::pmap seems to strip date class.

print_date = function(b) b

b = as.Date("2012-03-22")

purrr::pmap(list(b), print_date)
#> [[1]]
#> [1] 15421

Not sure if this is intended? Caught me out though :)

@david-jankoski
Copy link

@david-jankoski david-jankoski commented Aug 1, 2017

there were a couple of similar discussions around map and dates around here. this workaround does the job for me.

print_date = function(b) as.Date(b, origin = "1970-01-01")
b = as.Date("2012-03-22")
purrr::pmap(list(b), print_date)

[[1]]
[1] "2012-03-22"

hth.

@AkhilGNair
Copy link
Author

@AkhilGNair AkhilGNair commented Aug 1, 2017

Thanks @david-jankoski - That's what I'm currently doing. Just wanted to make sure it was known, I admitted didn't look around to see if anything similar has been asked.

@mungojam
Copy link

@mungojam mungojam commented Nov 10, 2017

I hit the same problem with pwalk

@huftis
Copy link
Contributor

@huftis huftis commented Nov 10, 2017

The bug is not specific to dates; pmap() removes all classes. Note that the plain map() function does not have this bug. Simple reprex:

library(purrr)
x = "test"
class(x) = "foo"
pmap(list(x), identity)
#> [[1]]
#> [1] "test"
map(list(x), identity)
#> [[1]]
#> [1] "test"
#> attr(,"class")
#> [1] "foo"

And as @mungojam mentioned, pwalk() also has this bug:

> pwalk(list(x), ~ print(class(.)))
# [1] "character" # should have been "foo" ...
@lionel-
Copy link
Member

@lionel- lionel- commented Nov 10, 2017

This is more of a known limitation rather than a bug, but we have plans to fix this.

@mungojam
Copy link

@mungojam mungojam commented Dec 11, 2017

I found today that flatten does this too and removed my Date class as well as doing the intended un-listing. Perhaps that is intentional?

@lionel-
Copy link
Member

@lionel- lionel- commented Dec 12, 2017

It is not intentional we've just been too busy with tidy eval and other projects :(

@mungojam
Copy link

@mungojam mungojam commented Dec 12, 2017

No worries, I'm enjoying all your other work :)

@hadley
Copy link
Member

@hadley hadley commented Feb 5, 2018

The root cause is this behaviour:

x <- as.Date("2012-03-22")
x[[1]]
#> [1] "2012-03-22"
list(x)[[c(1,1)]]
#> [1] 15421

This seems like a base R problem?

@hadley hadley added the bug label Feb 5, 2018
@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Feb 5, 2018

This seems like a base R problem?

It seems so to me; [[, or do_subset2(), tries to dispatch the method based only on the first argument: https://github.com/wch/r-source/blob/58aec3ee94f2a0769974520f6cb2d99c49fb829f/src/main/subset.c#L894-L914

If it fails, the default implementation of subset2 will be used for all subsets, but, IIUC, it doesn't care about the classes or attributes: https://github.com/wch/r-source/blob/58aec3ee94f2a0769974520f6cb2d99c49fb829f/src/main/subset.c#L1029-L1056

@czeildi
Copy link
Contributor

@czeildi czeildi commented May 1, 2018

Related: I'd love to see a map_date and map_datetime functions. My use case: there are some non-vectorized datetime related functions, e.g. lubridate::with_tz and I need map to create a new column with them in a data frame. I only found a workaround to arrive a plain datetime column.

library(purrr)
library(lubridate)
library(tibble)
library(dplyr)

df <- tibble(
  dttm_orig = ymd_hms(c("2018-04-01 08:00:00", "2017-03-02 03:00:00")),
  tz = c("UTC", "CET"),
  sales_amount = c(34, 56)
)

mutate(df, purchase_time = map2(dttm_orig, tz, ~with_tz(.x, .y)))
#> # A tibble: 2 x 4
#>   dttm_orig           tz    sales_amount purchase_time
#>   <dttm>              <chr>        <dbl> <list>       
#> 1 2018-04-01 08:00:00 UTC           34.0 <dttm [1]>   
#> 2 2017-03-02 03:00:00 CET           56.0 <dttm [1]>

mutate(df, purchase_time = map2_chr(dttm_orig, tz, ~as.character(with_tz(.x, .y))) %>% ymd_hms())
#> # A tibble: 2 x 4
#>   dttm_orig           tz    sales_amount purchase_time      
#>   <dttm>              <chr>        <dbl> <dttm>             
#> 1 2018-04-01 08:00:00 UTC           34.0 2018-04-01 08:00:00
#> 2 2017-03-02 03:00:00 CET           56.0 2017-03-02 04:00:00

Created on 2018-05-01 by the reprex package (v0.2.0).

I realize this might not be top priority however I would love to vote up an issue dedicated to this specifically.

@batpigandme
Copy link
Member

@batpigandme batpigandme commented May 1, 2018

however I would love to vote up an issue dedicated to this specifically.

@czeildi, if there isn't one, it sounds like you might want to open this as its own issue 👍

@hadley
Copy link
Member

@hadley hadley commented May 5, 2018

To resolve this issue, we need to make sure to file a bug in R's bugzilla.

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented May 5, 2018

Just out of curiosity, why is this not just about replacing .l[[c(j, i)]] with .l[[j]][[i]] here?

purrr/src/map.c

Lines 189 to 192 in d0a8081

// Construct call like .l[[c(j, i)]]
SEXP j_ = PROTECT(Rf_ScalarInteger(j + 1));
SEXP ji_ = PROTECT(Rf_lang3(Rf_install("c"), j_, nj == 1 ? one : i));
SEXP l_ji = PROTECT(Rf_lang3(R_Bracket2Symbol, l, ji_));

(I'm afraid this is not a bug, since it's reasonable that the method dispatch happens only once for one method.)

@burchill
Copy link
Contributor

@burchill burchill commented May 8, 2018

It might already be known, but calling as_vector() on a list of dates also strips Date.

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented May 9, 2018

@burchill Though it seems similar, I feel it's a separate issue. Maybe you should file a new issue. (BTW, I guess what you need is not as_vector(), but something like flatten_vec(). I expect _vec-family will be implemented in future. e.g. map_vec() on #499)

@mikmart
Copy link
Contributor

@mikmart mikmart commented Sep 26, 2018

Just out of curiosity, why is this not just about replacing .l[[c(j, i)]] with .l[[j]][[i]] here?

@yutannihilation I was curious, too, and tested making that change in this branch. As far as I can tell that should fix this issue; or at least the test suite passes and the original simple example is resolved.

@hadley
Copy link
Member

@hadley hadley commented Sep 26, 2018

@mikmart oh good idea. Want to do a PR?

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Sep 27, 2018

Thanks for the PR, @mikmart!

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

Successfully merging a pull request may close this issue.

None yet