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.

Sign up for GitHub

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 5 commits into from
Jan 18, 2024
+67 −81
Merged

fix check warnings/notes in assim.batch package #3200

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ede6ff5
fix notes/warnings in assim.batch package
moki1202 Jul 22, 2023
687d97f
add check.log file
moki1202 Jan 4, 2024
2bbb8c6
Merge branch 'develop' into assimBatch_notes_fix
infotroph Jan 4, 2024
ebd20c2
Update modules/assim.batch/tests/Rcheck_reference.log
mdietze Jan 17, 2024
b0d7790
Merge branch 'develop' into assimBatch_notes_fix
mdietze Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 30 additions & 76 deletions modules/assim.batch/tests/Rcheck_reference.log
View file
Open in desktop
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
Expand Down Expand Up @@ -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’
Copy link
Member

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?

Copy link
Collaborator Author

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.

Copy link
Member

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 and get.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.

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’:
Expand All @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The 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.


Loading