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

Change to pass data.table v1.12.0 please #85

Merged
merged 1 commit into from
Jan 6, 2019

Conversation

mattdowle
Copy link
Contributor

@mattdowle mattdowle commented Jan 4, 2019

Hi Alex,

data.table v1.12.0 is about to go to CRAN and revdep checking shows one fail in SpaDES.core. I've checked this PR passes. The 6th column of by= here is "arguments" which is type list which isn't supported by sorting/grouping/unique in data.table. It worked before because data.table didn't look at the type of the column until it needed to. In this case, the first few columns of by= were enough to establish the uniqueness and the subsequent columns weren't used. The new version of data.table works a bit differently and all the column types are now checked up-front.

The status before this PR was :

Quitting from lines 637-677 (ii-modules.Rmd) 
Error: processing vignette 'ii-modules.Rmd' failed with diagnostics:
Column 6 of by= (5) is type 'list', not yet supported
Execution halted

* checking PDF version of manual ... OK
* DONE

Status: 1 WARNING
See
  ‘/home/mdowle/build/revdeplib/SpaDES.core.Rcheck/00check.log’
for details.

Please could you update SpaDES.core on CRAN. Either in advance or after data.table is there is fine too.

Thanks, Matt

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 73.447% when pulling c9d259e on mattdowle:master into f9696fc on PredictiveEcology:master.

@PredictiveEcology PredictiveEcology deleted a comment from lintr-bot Jan 4, 2019
@achubaty
Copy link
Contributor

achubaty commented Jan 4, 2019

@eliotmcintire can you please review?

@achubaty achubaty requested a review from eliotmcintire January 4, 2019 21:12
@eliotmcintire
Copy link
Contributor

I think it is fine to accept PR.

Copy link
Contributor

@eliotmcintire eliotmcintire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine and appropriate

@eliotmcintire eliotmcintire merged commit b01f1ca into PredictiveEcology:master Jan 6, 2019
@achubaty
Copy link
Contributor

achubaty commented Jan 7, 2019 via email

eliotmcintire added a commit that referenced this pull request Jan 7, 2019
Change to pass data.table v1.12.0 please
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.

4 participants