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

reigning in examples / run-donttest #124

Closed
brownag opened this issue Jan 24, 2020 · 3 comments
Closed

reigning in examples / run-donttest #124

brownag opened this issue Jan 24, 2020 · 3 comments
Assignees

Comments

@brownag
Copy link
Member

brownag commented Jan 24, 2020

I finally got all soilDB examples wrapped in \donttest{} running successfully with R CMD CHECK --run-donttest

These are our big time consumers:

   Examples with CPU or elapsed time > 5s
                           user system elapsed
   fetchHenry              8.09   0.22   19.19
   us_ss_timeline          6.17   0.58  103.14
   fetchSDA_component      2.66   0.06   11.08
   fetchNASISWebReport     2.57   0.11    6.06
   mapunit_geom_by_ll_bbox 1.50   0.08   11.52
   fetchSDA_spatial        1.34   0.05    6.28

Summary: Many examples that rely on outside packages (aqp, ggplot2, sp, rgdal) needed to be using something heavier duty than requireNamespace() -- requireNamespace was only checking if the namespace could be loaded, not loading it. So many, many errors resulted from not using :: to access external methods. I used if-statement check on require() for each example dependency. Some packages are not dependencies of soilDB -- just suggests -- and using :: notation would be very maddening to look at in the case of e.g. ggplot2-reliant plots.

Furthermore, many functions that hit the database rely on some external datasource (e.g. selected set, PedonPC database) and needed to fail gracefully in the event of no data or an error in the request.

In general, I think we could dramatically clean up examples to make them simpler and easier to maintain. The more complex ones are essentially tutorials unto themselves. I made this issue to tag the relevant commits and make a note that we need to run R CMD CHECK with --run-donttest more frequently. There was a lot of out of date stuff in there.

@dylanbeaudette
Copy link
Member

Thanks Andrew, excellent work. A good reminder to keep examples as simple as possible.

@dylanbeaudette
Copy link
Member

Adding @smroecker and @jskovlin to the conversation: per short discussion with @brownag this morning:

  • move all examples that require > 5 seconds run-time out of the examples
  • inventory soilDB related tutorials, examples, etc.
  • organize soilDB content via bookdown or related system
  • provide links to extended examples in manual pages

@brownag
Copy link
Member Author

brownag commented Jan 16, 2021

donttest examples have been run regularly over the last year. We have even more longer-running examples than we did in the past, but that is OK -- they are stable and the logic to turn them off on test machines / in absence of suggested packages has been working.

   Examples with CPU (user + system) or elapsed time > 5s
                             user system elapsed
   taxaExtent              14.075  2.430  30.805
   SDA_spatialQuery         8.872  0.128  35.050
   fetchHenry               5.235  0.301  12.424
   fetchSDA_spatial         3.440  0.044   8.909
   fetchSDA_component       2.943  0.016  21.021
   fetchNASISWebReport      2.895  0.024  18.925
   seriesExtent             1.848  0.008   5.154
   mapunit_geom_by_ll_bbox  1.773  0.048  32.971
   fetchSCAN                0.556  0.044   9.146
   fetchKSSL                0.506  0.000  10.607
   fetchOSD                 0.340  0.008   7.097
   uncode                   0.342  0.000  11.553
   SDA_query                0.209  0.008   6.528
   fetchRaCA                0.170  0.016  13.620
   fetchSoilGrids           0.178  0.003  10.275

Closing this issue for CRAN release #155, and created a new issue to address @dylanbeaudette's points #157

@brownag brownag closed this as completed Jan 16, 2021
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

No branches or pull requests

2 participants