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

Retain attributes when combining cran_deps + remote_deps #300

Closed
wants to merge 4 commits into from

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Feb 20, 2019

cran_deps bears the type attribute

This fixes the problem I described in slack.

The order of arguments in rbind() affects the attributes of the resulting data frame.

a <- data.frame(a = 1)
attr(a, "yo") <- "hi"

b <- data.frame(a = 2)

attr(rbind(a, b), "yo")
#> [1] "hi"
attr(rbind(b, a), "yo")
#> NULL

Created on 2019-02-19 by the reprex package (v0.2.1.9000)

I don't know why other people aren't seeing this, so perhaps there is more going on? But this fixes things for me.

cran_deps bears the `type` attribute
@jennybc jennybc force-pushed the preserve-type-attribute branch from d664b31 to 5b86a48 Compare February 20, 2019 02:56
@jennybc
Copy link
Member Author

jennybc commented Feb 20, 2019

This PR passed tests (with 1 skipped test) everywhere but R-devel on travis, so I think that difficulty is orthogonal to this.

@jennybc
Copy link
Member Author

jennybc commented Feb 20, 2019

Maybe getting closer to the root cause ...

I think this row indexing is where the type attribute can potentially be lost from either the cran or gh deps.

remotes/R/deps.R

Lines 148 to 152 in 6212345

# Only keep the remotes that are specified in the cran_deps or are NA
remote_deps <- remote_deps[is.na(remote_deps$package) | remote_deps$package %in% cran_deps$package, ]
# If there are remote deps remove the equivalent CRAN deps
cran_deps <- cran_deps[!(cran_deps$package %in% remote_deps$package), ]

df <- data.frame(a = 1:3)
attr(df, "yo") <- "hi"
attr(df, "yo")
#> [1] "hi"
attr(df[1, ], "yo")
#> NULL

Created on 2019-02-20 by the reprex package (v0.2.1.9000)

So my fix of inverting the order inside rbind() isn't the general solution. I think it just works out better in the examples I'm currently facing.

@jennybc jennybc changed the title Invert order to retain attributes from cran_deps Retain attributes when combining cran_deps + remote_deps Feb 21, 2019
@jennybc
Copy link
Member Author

jennybc commented Feb 24, 2019

Closing in favour of #304

@jennybc jennybc closed this Feb 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant