-
Notifications
You must be signed in to change notification settings - Fork 235
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
fix check warnings/notes in assim.batch package #3200
Merged
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
ede6ff5
fix notes/warnings in assim.batch package
moki1202 687d97f
add check.log file
moki1202 2bbb8c6
Merge branch 'develop' into assimBatch_notes_fix
infotroph ebd20c2
Update modules/assim.batch/tests/Rcheck_reference.log
mdietze b0d7790
Merge branch 'develop' into assimBatch_notes_fix
mdietze File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,62 +1,13 @@ | ||
* using log directory ‘/home/tanishq010/pecan/modules/PEcAn.assim.batch.Rcheck’ | ||
* using R version 4.2.1 (2022-06-23) | ||
* using log directory ‘/tmp/Rtmp0m7lr6/PEcAn.assim.batch.Rcheck’ | ||
* using R version 4.1.3 (2022-03-10) | ||
* using platform: x86_64-pc-linux-gnu (64-bit) | ||
* using session charset: UTF-8 | ||
* using options ‘--no-manual --as-cran’ | ||
* checking for file ‘PEcAn.assim.batch/DESCRIPTION’ ... OK | ||
* checking extension type ... Package | ||
* this is package ‘PEcAn.assim.batch’ version ‘1.7.2’ | ||
* this is package ‘PEcAn.assim.batch’ version ‘1.7.2.9000’ | ||
* package encoding: UTF-8 | ||
* checking CRAN incoming feasibility ... WARNING | ||
Maintainer: ‘Istem Fer <[email protected]>’ | ||
|
||
New submission | ||
|
||
License components with restrictions and base license permitting such: | ||
BSD_3_clause + file LICENSE | ||
File 'LICENSE': | ||
University of Illinois/NCSA Open Source License | ||
|
||
Copyright (c) 2012, University of Illinois, NCSA. All rights reserved. | ||
|
||
Permission is hereby granted, free of charge, to any person obtaining | ||
a copy of this software and associated documentation files (the | ||
"Software"), to deal with the Software without restriction, including | ||
without limitation the rights to use, copy, modify, merge, publish, | ||
distribute, sublicense, and/or sell copies of the Software, and to | ||
permit persons to whom the Software is furnished to do so, subject to | ||
the following conditions: | ||
|
||
- Redistributions of source code must retain the above copyright | ||
notice, this list of conditions and the following disclaimers. | ||
- Redistributions in binary form must reproduce the above copyright | ||
notice, this list of conditions and the following disclaimers in the | ||
documentation and/or other materials provided with the distribution. | ||
- Neither the names of University of Illinois, NCSA, nor the names | ||
of its contributors may be used to endorse or promote products | ||
derived from this Software without specific prior written permission. | ||
|
||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, | ||
EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF | ||
MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. | ||
IN NO EVENT SHALL THE CONTRIBUTORS OR COPYRIGHT HOLDERS BE LIABLE FOR | ||
ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF | ||
CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION | ||
WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS WITH THE SOFTWARE. | ||
|
||
Strong dependencies not in mainstream repositories: | ||
PEcAn.benchmark, PEcAn.DB, PEcAn.emulator, PEcAn.logger, PEcAn.MA, | ||
PEcAn.remote, PEcAn.settings, PEcAn.uncertainty, PEcAn.utils, | ||
PEcAn.workflow | ||
|
||
The Date field is over a month old. | ||
* checking package namespace information ... OK | ||
* checking package dependencies ... WARNING | ||
Imports includes 27 non-default packages. | ||
Importing from so many packages makes the package vulnerable to any of | ||
them becoming unavailable. Move as many as possible to Suggests and | ||
use conditionally. | ||
|
||
* checking package dependencies ... NOTE | ||
Imports includes 27 non-default packages. | ||
Importing from so many packages makes the package vulnerable to any of | ||
|
@@ -87,37 +38,17 @@ use conditionally. | |
* checking whether the namespace can be loaded with stated dependencies ... OK | ||
* checking whether the namespace can be unloaded cleanly ... OK | ||
* checking loading without being on the library search path ... OK | ||
* checking use of S3 registration ... OK | ||
* checking dependencies in R code ... NOTE | ||
Namespace in Imports field not imported from: ‘dplyr’ | ||
All declared Imports should be used. | ||
* checking dependencies in R code ... OK | ||
* checking S3 generic/method consistency ... OK | ||
* checking replacement functions ... OK | ||
* checking foreign function calls ... OK | ||
* checking R code for possible problems ... NOTE | ||
get.da.data: no visible binding for global variable ‘ensemble.samples’ | ||
get.da.data : <anonymous>: no visible binding for global variable | ||
‘sa.samples’ | ||
get.da.data: no visible binding for global variable ‘sa.samples’ | ||
get.da.data : <anonymous>: no visible global function definition for | ||
‘read.output.type’ | ||
get.da.data.growth: no visible binding for global variable | ||
‘ensemble.samples’ | ||
get.da.data.growth: no visible binding for global variable ‘sa.samples’ | ||
get.da.data.growth : <anonymous>: no visible global function definition | ||
for ‘read.output.type’ | ||
pda.load.priors: no visible binding for global variable ‘post.distns’ | ||
pda.load.priors: no visible binding for global variable ‘prior.distns’ | ||
plot.da: no visible binding for global variable ‘y’ | ||
plot.da: no visible binding for global variable ‘ensemble.samples’ | ||
plot.da : <anonymous>: no visible binding for global variable | ||
‘ensemble.samples’ | ||
plot.da : <anonymous>: no visible binding for global variable | ||
‘sa.samples’ | ||
plot.da : <anonymous>: no visible binding for global variable ‘m’ | ||
Undefined global functions or variables: | ||
ensemble.samples m post.distns prior.distns read.output.type | ||
sa.samples y | ||
read.output.type | ||
|
||
Found the following assignments to the global environment: | ||
File ‘PEcAn.assim.batch/R/pda.get.model.output.R’: | ||
|
@@ -140,9 +71,32 @@ File ‘PEcAn.assim.batch/R/pda.get.model.output.R’: | |
OK | ||
* checking for unstated dependencies in vignettes ... OK | ||
* checking package vignettes in ‘inst/doc’ ... OK | ||
* checking re-building of vignette outputs ... OK | ||
* checking re-building of vignette outputs ... WARNING | ||
Error(s) in re-building vignettes: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand that this gets us around the isFalse issue, but I don't feel great about switching a reference log from an OK to a WARNING (the idea of the log was to only move in the direction towards fewer warnings) |
||
... | ||
--- re-building ‘AssimBatchVignette.Rmd’ using rmarkdown | ||
Error: processing vignette 'AssimBatchVignette.Rmd' failed with diagnostics: | ||
The function xfun::isFALSE() will be deprecated in the future. Please consider using base::isFALSE(x) or identical(x, FALSE) instead. | ||
--- failed re-building ‘AssimBatchVignette.Rmd’ | ||
|
||
--- re-building ‘MultiSitePDAVignette.Rmd’ using rmarkdown | ||
Error: processing vignette 'MultiSitePDAVignette.Rmd' failed with diagnostics: | ||
The function xfun::isFALSE() will be deprecated in the future. Please consider using base::isFALSE(x) or identical(x, FALSE) instead. | ||
--- failed re-building ‘MultiSitePDAVignette.Rmd’ | ||
|
||
SUMMARY: processing the following files failed: | ||
‘AssimBatchVignette.Rmd’ ‘MultiSitePDAVignette.Rmd’ | ||
|
||
Error: Vignette re-building failed. | ||
Execution halted | ||
|
||
* checking for non-standard things in the check directory ... OK | ||
mdietze marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* checking for detritus in the temp directory ... OK | ||
* DONE | ||
|
||
Status: 2 WARNING, 3 NOTEs | ||
Status: 1 WARNING, 2 NOTEs | ||
See | ||
‘/tmp/Rtmp0m7lr6/PEcAn.assim.batch.Rcheck/00check.log’ | ||
for details. | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 required for the PR, but I'm curious what's up with
read.output.type
and why this one undefined variable was left?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.
@mdietze I think I remember this. I was unable to find which package does read.output.type function belong to.
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.
Looking at those functions, I suspect
get.da.data.growth.R
andget.da.data.R
can both be moved from the/R
folder to the/inst
folder. I don't have time to go through all the code in detail, but I think most of what's here is redundant with newer code by @istfer and others. This code doesn't appear to be operationally linked to other bits of the PDA and the value of retaining this code is primarily because it provides a record of what Carl Davidson did in his (still unpublished) Master thesis.