-
Notifications
You must be signed in to change notification settings - Fork 6
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
Move arrow to Suggests to appease CRAN #40
Conversation
@@ -1,7 +1,9 @@ | |||
*.Rcheck/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are left behind by R CMD check
and R CMD build
, respectively, so I added them to .gitignore
@@ -41,3 +41,4 @@ VignetteBuilder: | |||
Encoding: UTF-8 | |||
Roxygen: list(markdown = TRUE) | |||
RoxygenNote: 7.2.3 | |||
Additional_repositories: https://p3m.dev/cran/2024-02-02/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: CRAN acknowledges this field but doesn't use it AFAIK. install.packages()
does not use it. remotes::install_*()
does use it, but pak::pak()
apparently does not (yet) 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the clarification
error = function(e){ | ||
msg <- paste( | ||
"File cached locally seems to be corrupted. Please download it again using 'cache = FALSE'.", | ||
sprintf("Alternatively, you can remove the corrupted file with 'censobr::censobr_cache(delete_file = \"%s\")'", filename), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One benefit of moving this error handling here: we have the filename inside this function, so we can insert it into the error message that suggests what to run to delete the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clever. Thanks
@@ -3,6 +3,7 @@ context("add_labels_emigration") | |||
# skip tests because they take too much time | |||
skip_if(Sys.getenv("TEST_ONE") != "") | |||
testthat::skip_on_cran() | |||
testthat::skip_if_not_installed("arrow") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly necessary for CRAN because these are already skipped there, but in case you're running locally without arrow
@@ -26,7 +27,7 @@ test_that("read_emigration", { | |||
|
|||
# add labels | |||
test4 <- read_emigration(add_labels = 'pt', columns = c('abbrev_state', 'V1006')) | |||
test4 <- test4 |> filter(abbrev_state == 'CE') |> as.data.frame() | |||
test4 <- test4 |> dplyr::filter(abbrev_state == 'CE') |> as.data.frame() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was running individual files to test and dplyr
wasn't loaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice catch
Hi @nealrichardson. This is all very much appreciated. I've reviewed the proposed changes and they look Ok to me. Thanks for your contributions! |
Mas agora fiquei curiso sobre sua tese de doutorado! Is there any link where I could find it ? |
É disponível no Proquest, pelo menos a introduçāo: https://www.proquest.com/openview/37dbfb75fe29ce81f27f8e8ec67a68be/1?pq-origsite=gscholar&cbl=18750 Este artigo também utilizou muitos dados do IBGE: https://direct.mit.edu/rest/article-abstract/92/3/505/57828/Economic-Determinants-of-Land-Invasions |
ah! this looks great! Thanks for sharing! |
Related to apache/arrow#39806. Since it's not clear that we'll be able to resolve everything with CRAN before February 9, this PR moves arrow from
Imports
toSuggests
and checks for the availability of arrow before callingopen_dataset()
. This should passR CMD check --as-cran
Alem disso, obrigado pelo
censobr
! No meu doutorado (muitos anos atrás), eu fiz várias pesquisas com dados do IBGE e do Ipea. Teria sido muito mais fácil se tivesse ocensobr
naquela época!