Package Review and CMD Check

[BatoolMM]

As mentioned in #22, we need to address two issues to make sure all function work before creating any docs:

• Debug or make sure that domain_mapping() function is working Current error:

  Error in fromJSON(file = json_file) : 
  either json_str or file must be supplied to fromJSON

• Make sure all check pass in CMD check Current errors:

  
  W  checking dependencies in R code ...
  'library' or 'require' calls not declared from:
   ‘grid’ ‘gridExtra’ ‘insight’ ‘rjson’
  'library' or 'require' calls in package code:
   ‘grid’ ‘gridExtra’ ‘insight’ ‘rjson’
   Please use :: or requireNamespace() instead.
   See section 'Suggested packages' in the 'Writing R Extensions

N checking R code for possible problems (4.7s) domain_mapping: no visible global function definition for ‘data’ domain_mapping: no visible binding for global variable ‘json_metdata’ domain_mapping: no visible binding for global variable ‘domains_list’ domain_mapping: no visible global function definition for ‘fromJSON’ domain_mapping: no visible global function definition for ‘read.csv’ domain_mapping: no visible global function definition for ‘plot.new’ domain_mapping: no visible global function definition for ‘grid.table’ domain_mapping: no visible global function definition for ‘print_colour’ domain_mapping: no visible global function definition for ‘write.csv’ Undefined global functions or variables: data domains_list fromJSON grid.table json_metdata plot.new print_colour read.csv write.csv Consider adding importFrom("graphics", "plot.new") importFrom("utils", "data", "read.csv", "write.csv") to your NAMESPACE file.

W checking for missing documentation entries ... Undocumented code objects: ‘domains_list’ ‘json_metdata’ Undocumented data sets: ‘domains_list’ ‘json_metdata’ All user-level objects in a package should have documentation entries. See chapter ‘Writing R documentation files’ in the ‘Writing R Extensions’ manual.

[BatoolMM]

This is + traceback:

Error in fromJSON(file = json_file) : 
  either json_str or file must be supplied to fromJSON
3. stop("either json_str or file must be supplied to fromJSON")
2. fromJSON(file = json_file) at domain_mapping.R#38
1. domain_mapping()

[BatoolMM]

@RayStick Very stupid question but am I supposed to provide the json_file inside the function when I run the function.

I think I have been running it without json_file!

[RayStick]

Let me check :D

[RayStick]

Do you run the steps like this for the demo?

[BatoolMM]

This is what I have been running:

library(devtools)
devtools::load_all()
?domain_mapping
domain_mapping()

but now that I run:

domain_mapping(,, TRUE)

it's working, yah!!!

Another question, is there a reason why the 3rd argument isn't by default TRUE, is this to avoid another issue?

[RayStick]

Great =D

What this TRUE means is demo_mode = TRUE The normal way to run it would be domain_mapping(json_file, domain_file) with the last argument by default FALSE because it is not in demo mode. For demo mode, it does not need any arguments but you need to tell it to run it as demo mode by changing demo_mode = TRUE.

Does that make sense? Do you think there is a better way to approach it?

[RayStick]

Maybe I can code something that says if no input arguments are given, it triggers demo mode. And remove this argument altogether?

[BatoolMM]

It makes total sense - thank you for the clarification!

I am trying to recall examples where R packages have similar approach, it's usually the 1st argument that is used to trigger example. Let me sleep on this and see what is best practices!

I am very happy that it's working now!

[RayStick]

Yes, we can return to this, no rush. Would be good to do what is typical for an R package, so let me know!

[BatoolMM]

I am looking into the 2nd point with CMD check - no need to reply today - I am just adding my points and questions to come back to it.

I don't see that any of the dependencies mentioned in DESCRIPTION file. Also, I would also use :: always when recall any of the packages inside a new function. I usually don't do that in regular script but inside a new function within a new pkg, it's highly recommended.

[RayStick]

To summarize a few points from our call. Before public release we want:

• to simplify the package installation (auto install required dependencies)
• to simplify how the demo run is called e.g. just domain_mapping

Perhaps a PR for each?

[BatoolMM]

Thank you, yes, it makes sense to have 2 PRs.