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

Enhancement#305 gridded global datav5 #323

Open
wants to merge 76 commits into
base: master
Choose a base branch
from
Open

Enhancement#305 gridded global datav5 #323

Changes from 1 commit
Commits
Show all changes
76 commits
Select commit Hold shift + click to select a range
23b6834
Started integrating new soil data (100m)
nip5 Jun 26, 2018
33a4e93
extracts sand/clay/bd successfully
nip5 Jul 5, 2018
8943c9b
Integrated gravel data from 250m
nip5 Jul 5, 2018
cdd2294
Refactored code to run without as much user guidance
nip5 Jul 6, 2018
f49e3e4
Improved some soil calculations, added some environment changes
nip5 Jul 6, 2018
680ddfe
Moved the assignment in certain fields out of new function to be more…
nip5 Jul 9, 2018
8497ff5
starting integrating 250m depth
nip5 Jul 11, 2018
20341ed
made adjustments for rSFSW2 tests
nip5 Jul 12, 2018
246d291
some performance enhancements
nip5 Jul 16, 2018
bcdebb4
started merge with decadel aggregations
nip5 Jul 16, 2018
6063ecf
Merged with feature_DecadelAggregations, implemented cell soil extrac…
nip5 Jul 17, 2018
0eca671
Removed database comparisions in TestPrj4 to allow passing of all uni…
nip5 Jul 18, 2018
c1e8dc9
Added unit test file for testing soil extraction related functions
nip5 Jul 19, 2018
c14507c
Added unit tests for get_datasource_masterfield and get_datasource_in…
nip5 Jul 19, 2018
5a1cee0
Added test for prepare_ExtractData_Soils
nip5 Jul 19, 2018
41630f9
Some print statements only used during development
nip5 Jul 20, 2018
b1df9f5
Added Roxygen documentation to do_ExtractSoilDataFrom100m
nip5 Jul 20, 2018
6af2dc7
Remove extra comment weight
nip5 Jul 20, 2018
9229243
Fix issue with unit tests getting hung up after conversion from = to <-
nip5 Jul 20, 2018
4c0bf91
Fix extracted gravel data being unusually low
nip5 Jul 23, 2018
e7d332e
Add depth tif integration
nip5 Jul 23, 2018
1a75875
Add unit test for update_soils_input
nip5 Jul 23, 2018
91d1f53
Add unit test for new extraction function
nip5 Jul 24, 2018
575e35b
Fix testPrj4 now works again
nip5 Jul 24, 2018
bea0f35
Add resume functionality to function
nip5 Jul 25, 2018
71b714b
Remove print statements and debug comments
nip5 Jul 25, 2018
7ae6713
Change back models flag for CI tests
nip5 Jul 25, 2018
367b6aa
Remove 250m extaction unit test for CI
nip5 Jul 26, 2018
d73f71b
Delete .RData
nip5 Jul 26, 2018
0775cb0
Delete rSFSW2.Rproj
nip5 Jul 26, 2018
4b76ef7
Remove unecessary files
nip5 Jul 26, 2018
e3e37a4
Merge branch 'Enhancement#305_GriddedGlobalDatav5' of https://github.…
nip5 Jul 26, 2018
5f9f3b3
comment out extract soil unit tests for CI debugging
nip5 Jul 26, 2018
dbe6fb0
Remove extract soil data unit test
nip5 Jul 26, 2018
77dcc0f
Lots of code cleanup
nip5 Jul 26, 2018
684481b
Modify verbose messages to only show once each
nip5 Jul 26, 2018
0cbec09
Add layers done info when in verbose mode
nip5 Jul 26, 2018
afc2c78
Add test file back in but skip on CIs
nip5 Jul 27, 2018
364edc9
Change NRCS to Isric source
nip5 Aug 6, 2018
bc1c92a
Merge branch 'master' into Enhancement#305_GriddedGlobalDatav5
nip5 Aug 6, 2018
4b87610
Modify to improve code coverage
nip5 Aug 7, 2018
ccd634d
Several spacing changes, add files back in from master
nip5 Aug 13, 2018
49bb5a2
modify to address review
nip5 Aug 13, 2018
d4caa3a
modify spacnig to be standardized with master
nip5 Aug 13, 2018
f6e42ef
fix to address review
nip5 Aug 14, 2018
9e35388
add man/rSFSW2.Rd
nip5 Aug 14, 2018
383f251
fix program crashing if run when soils data was already available
nip5 Aug 17, 2018
7bf10da
more elegant fix to previous commit
nip5 Aug 17, 2018
0b4d749
performance enhancements
nip5 Aug 17, 2018
1ae4703
modify for review and staying <= 80 chars per line
nip5 Aug 20, 2018
304e609
fix spelling and format test failing
nip5 Aug 24, 2018
29981ff
fix soil extract test failing
nip5 Aug 27, 2018
3bbfdda
remove test_ExtractData_Soils.R, this branch is ready for review
nip5 Aug 27, 2018
b21e3e3
fixes to address code review
nip5 Sep 10, 2018
b28734c
Update package-level linter settings
dschlaep Sep 12, 2018
1b1a3fd
add performance enhancements
nip5 Sep 12, 2018
79c758b
Merge branch 'Enhancement#305_GriddedGlobalDatav5' of https://github.…
nip5 Sep 12, 2018
87101ff
fix for crashing and lintr failing
nip5 Sep 12, 2018
f5b3473
merge with master
nip5 Jan 30, 2019
9e87219
lintr compliance fixes
nip5 Jan 30, 2019
2b163e3
moved verbose message out of main loop
nip5 Jan 30, 2019
f1bb913
modify extract_soilISRIC250m function description
nip5 Jan 30, 2019
be96b2c
modify MMC usage in ISRIC extraction function
nip5 Jan 30, 2019
2be512b
add extract soil test with on/off flag
nip5 Jan 30, 2019
3731d90
Merge branch 'master' into Enhancement#305_GriddedGlobalDatav5
nip5 Mar 8, 2019
1e62d13
add catch if file to extract is found but has bad content
nip5 Mar 8, 2019
6c772c7
Merge branch 'master' into Enhancement#305_GriddedGlobalDatav5
nip5 Mar 8, 2019
ca1f319
Merge branch 'Enhancement#305_GriddedGlobalDatav5' of https://github.…
nip5 Mar 8, 2019
4e9a4cb
lintr compliance
nip5 Mar 8, 2019
349a4e8
remove semicolons in extract soils unit test
nip5 Mar 13, 2019
2d978b8
add spaces between ) and { in some instances
nip5 Mar 13, 2019
b5daa1d
fix odd indenting
nip5 Mar 13, 2019
df084f4
pulled out percent_div to set it as a default value
nip5 Mar 13, 2019
1696fda
remove SSURGO soils data flag as it is not implemented
nip5 Mar 13, 2019
9226360
change CI fix format
nip5 Mar 14, 2019
5f2c10d
add check for file configurations in unit test
nip5 Mar 14, 2019
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
16 changes: 10 additions & 6 deletions tests/testthat/test_ExtractData_Soils.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ context("Soil data extraction")
# - "Error: On Travis"
# Check values of the ENV variables directly as a work-around:

# whether or not these tests should be run on the CIs
do_skip <- c(
!identical(tolower(Sys.getenv("RSFSW2_ALLTESTS")), "true"),
# whereas skip_on_cran() skips if not "true", I believe it should skip only
Expand All @@ -18,23 +19,26 @@ do_skip <- c(
# mimmics skip_on_appveyor():
identical(tolower(Sys.getenv("APPVEYOR")), "true"))

suppressWarnings(is_online <-
!inherits(try(close(url(getOption("repos"), open = "r")), silent = TRUE),
"try-error"))


if (!any(do_skip) && is_online) {
if (!any(do_skip)) {
# =============================================================================
# Tests designed to test the underlining structures created
# from soil extraction functions.
# =============================================================================

# set stage manually so that future changes won't cause the test to fail ======
# setup file paths
fnames_in <- environment()
fnames_in$fslayers <- file.path("/home/natemccauslin/Desktop/Dryland Ecology/rSFSW2/tests/test_data/TestPrj4/1_Input/SWRuns_InputData_SoilLayers_v9.csv")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why absolute paths for fslayers and fsoils specific to your computer? The folder TestPrj4 is part of this package source, thus, it will always be the same and you should use a relative path, e.g., see example in test_projects.R which uses dir_tests <- file.path("..", "test_data", "TestPrj4")

fnames_in$fsoils <- "/home/natemccauslin/Desktop/Dryland Ecology/rSFSW2/tests/test_data/TestPrj4/1_Input/datafiles/SWRuns_InputData_soils_v12.csv"
# directory where extertnal soils data is located
dir_ex_soil <- "/media/natemccauslin/SOILWAT_DATA/GIS/Data/Soils/"

# if any of the file paths above are invalid, skip all tests and setup
if (!all(file.exists(c(fnames_in$fslayers, fnames_in$fsoils, dir_ex_soil)))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't check that dir_ex_soil contains the required content for function extract_soil_ISRIC250m. Let's assume that dir_ex_soil exists but the person has not downloaded the specific dataset, then this still fails because of

dir.ex.gridded <- file.path(dir_ex_soil, "ISRIC", "GriddedGlobalV5")
  # stop program execution if folder path is incorrect
  if (!dir.exists(dir.ex.gridded)) stop(paste0("Folder '", dir.ex.gridded,
"' does not exist"))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The unit test shouldn't be testing if the directory contains the files before the function is called because that is done in extract_soil_ISRIC250m so it is done regardless of how the function is being called. Basically if I were to do that I'd be checking it twice.

  • Having the function fail because the person doesn't have the correct dataset is by design.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the function fail because a user doesn't have the files is correct; but having the unit tests fail because a developer doesn't have the files is bad -- the unit tests should skip.

skip("File paths incorrectly configured for Soils Extraction tests")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm repeating myself skip doesn't work outside test_that blocks.

Why don't you run the package tests on another machine that doesn't have GriddedGlobalV5 installed? Something like

Sys.setenv(NOT_CRAN = "true")
Sys.setenv(RSFSW2_ALLTESTS = "true")
devtools::test()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to work, do you mean you don't like it outside of testthat?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct skip does work correctly outside of test_that blocks -- this must be a new feature of more recent testthat versions because it didn't work in earlier versions when I checked it out originally several years back (e.g., discussion on this r-lib/testthat@b1e41a0).

Apparently, the devel version of testthat also introduced an skip_if_offline (r-lib/testthat@be8e6b6) which will be nice for us!

Ok, I am fine with your skips, but there is still the problem that the tests are not skipped and fail if the correct external files are not available.

}

# set stage manually so that future changes won't cause the test to fail ======
resume <- TRUE
verbose <- FALSE
MMC <- environment()
Expand Down