release v0.1.4 about r3pg HOT 14 CLOSED

It's done! I ran the CRAN checks again, and now there is only one warning:
WARNING 'qpdf' is needed for checks on size reduction of PDFs
I'm sending you the pull request.

from r3pg.

Hi @trotsiuk !

yes, no problem, but I am not totally sure if I understand this bit right:

please use (potentially modified) already existing internal data for var.default - i_parameters, sizeDist.default - d_sizeDist, param.default - d_parameters instead creating the new one in sysdata.rda

Do you mean that I should save d_param_lit in the visible data, instead of in the internal data? Do you want the code in 0_create_input_data.R or 0_create_internal_data.R?

from r3pg.

Yes, correct @Rasilgon . I would suggest to make the d_param_lit (or whatever you name it) visible and document in the data.R. And as mentioned, use (modify) existing d_.. tables for those purpose.
I hope this is ok from your side, and sorry that I didn't realise it during the merge request.
I guess it is easier to put everything into one file (e.g. 0_create_input_data.R ) but you can also keep it separately (0_create_internal_data.R). Even those if you will be modifying d_.. tables it is easier to do everything in one file (e.g. 0_create_input_data.R )

from r3pg.

No worries, I totally agree. I didn't think of it before. I was not that aware of the pkg structure.
If the d_param_lit is the full version of the parameter database (parameter, source, and comments), there is no need of modifying any existing d_.. table, I'd say. The get_parameter function would return subset or queries of d_param_lit.
Or do you mean to append all parameter sets without source and comments to the d_parameters?
If so, get_parameter is perhaps not necessary. I mean, the d_param_lit is already visible, and a run-ready version of it will be in d_parameters.

from r3pg.

I have seen that you also created the var.default, sizeDist.default, param.default tables which are similar/identical to i_parameters, d_sizeDist, d_parameters and thus would suggest to remove the ...defaul. If you are using them in any of the script you can replace them with d_... tables.

from r3pg.

Hi @trotsiuk,
Yes, I can remove them, although I thought that they were meant to exist.
I ran the 0_create_internal_data.R script, which creates the var.default, sizeDist.default, param.default tables and saves them as internal data. Saving the defaults tables was part of the original code, so I assumed that it had to be run.
usethis::use_data(var.default, param.default, sizeDist.default, param.db, internal = TRUE, overwrite = TRUE)
I only added the param.db . and committed it along with the internal data, to keep it updated.

from r3pg.

You are right. I totally confused myself :) I will then check whether we can potentially get rid of those system data. Sorry for confusion

from r3pg.

No worries! Let me know when you are done with it, and then I will go through my code and fix it accordingly. BTW, please feel free to modify it. My main contribution is to include the db, but you might better see how to fit it in the pkg.

from r3pg.

HI @Rasilgon, I have now made the changes.
Can you please:

• document i_parameters_lit (previously param.df) in the data.R (you might also consider splitting this list to separate data.frames, but not critical)
• check the data preparation script whether correct in 0_create_input_data.R
• write your information in the DESCRIPTION

Thanks in advance

from r3pg.

Hi @trotsiuk ,

sorry for the late reply, I did as you suggested 👍 The pull request is already sent.

from r3pg.

Hi @Rasilgon

There are two small issues with the modification

1. I think this shall be renamed

   r3PG/pkg/R/get_parameters.R

   Line 38 in 4448713

   #' @seealso \code{\link{run_3PG}}, \code{\link{i_parameters_list}}

   to _lit

Data codoc mismatches from documentation object 'i_parameters_lit':
Variables in data frame 'i_parameters_lit'

2. It is not directly taken the column names from other tables, either all need to be documented or a proper link made

  Code: BLcond CoeffCond D13CTissueDif Dlocation0 DlocationB DlocationC
        Dlocationrh Dlocationt Dscale0 DscaleB DscaleC Dscalerh Dscalet
        Dshape0 DshapeB DshapeC Dshaperh Dshapet LAIgcx LAImaxIntcptn
        MaxAge MaxCond MaxIntcptn MinCond Qa Qb RGcGw SLA0 SLA1 SWconst
        SWpower Tmax Tmin Topt Y aFracDiffu aH aHL aK aV aWS age
        alphaCx bFracRubi cVPD country crownshape fCalpha700 fCg700 fN0
        fNn fracBB0 fracBB1 fullCanAge gDM_mol gammaF0 gammaF1 gammaN0
        gammaN1 gammaR k kF leaffall leafgrow link m0 mF mR mS
        molPAR_MJ nAge nHB nHC nHLB nHLC nHLL nHLrh nKB nKC nKH nKrh
        nVB nVBH nVH nWS ngammaN notes pFS2 pFS20 pRn pRx parset_id
        rAge region rhoMax rhoMin source source_comments source_full
        species tBB tRho tSLA tgammaF tgammaN thinPower type wSx1000
        wslocation0 wslocationB wslocationC wslocationrh wslocationt
        wsscale0 wsscaleB wsscaleC wsscalerh wsscalet wsshape0 wsshapeB
        wsshapeC wsshaperh wsshapet year
  Docs: age country link notes parset_id region remaining columns
        source source_comments source_full species type year

from r3pg.

Hi @trotsiuk! Thanks for checking 👍, and sorry for the trouble.

1. I think this shall be renamed

Sorry, I misspelled the i_parameters_lit. Already fixed.

2. It is not directly taken the column names from other tables, either all need to be documented or a proper link made.

The remaining columns are the parameter names, which is the information in the column parameters from i_parameters and i_sizeDist. I think that listing all parameters in the data.R does not make much sense. I did not mean the code to take the column names from other table or the value of one column, I just wanted to describe the structure of the table i_parameters_list, and avoid listing all parameters.
From your comment, I figure, that this is not the way to do it and/or it is problematic. If so, I would remove this bit:
#' \item{remaining columns}{name of the parameters, consistent with naming in \code{\link{i_parameters}} or \code{\link{i_sizeDist}}}
and include this information in @details. Would this be a valid solution? Or would you suggest another one?

from r3pg.

Hi @Rasilgon

This is unfortunately, but we indeed need to list all the items for the table. Otherwise the CRAN submission will complain.
You can simply use Check Package during the build to see whether there are complains.

Thanks for understanding.

from r3pg.

Hi @trotsiuk !
Yes, I ran the crank checks, but as it is only a warning, I thought we could get away with it ;) No problem, I will include the full description using the information in i_parameters and i_sizeDist.

from r3pg.