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

fix met2CF.csv tests #202

Closed
dlebauer opened this issue Sep 2, 2014 · 10 comments · Fixed by #3301
Closed

fix met2CF.csv tests #202

dlebauer opened this issue Sep 2, 2014 · 10 comments · Fixed by #3301

Comments

@dlebauer
Copy link
Member

dlebauer commented Sep 2, 2014

Commented out test in data.atmosphere/tests/testthat/test.met2CF.csv.R

d> test("../../")
Testing PEcAn.data.atmosphere
Loading PEcAn.data.atmosphere
loading data from PEcAn-CF met drivers : ..........
downscaling : .......
testing csv import using met2CF.csv :
Error in ncvar_add(nc = nc, var = qair.var, verbose = nc_verbose) :
unused argument (var = qair.var)

These should be revised, extended, and fixed

robkooper added a commit that referenced this issue Sep 4, 2014
The file modules/data.atmosphere/inst/extdata/urbana_daily_test.nc has
the wrong start_date
@robkooper robkooper modified the milestone: 1.4 Benchmarking Oct 2, 2014
@mdietze mdietze modified the milestones: 1.4.1 Benchmarking: FACE sites, 1.4.0 Met workflow Nov 25, 2014
@mdietze mdietze modified the milestones: 1.4.2 Benchmarks, etc, 1.4.1 Regional Met Jan 16, 2015
@mdietze mdietze modified the milestones: 1.4.3 Benchmark calculations, 1.4.2 Finish Met Workflow Apr 16, 2015
@mdietze mdietze modified the milestones: 1.4.5: Benchmark workflow, 1.4.4 Sep 29, 2015
@mdietze mdietze modified the milestones: 1.4.6 CLM, 1.4.5: CRUNCEP, Preles Jan 25, 2016
@mdietze mdietze modified the milestones: On Deck, 1.4.7 Flux Camp 2016 Jun 2, 2016
@robkooper robkooper modified the milestone: On Deck Apr 14, 2020
@github-actions
Copy link

This issue is stale because it has been open 365 days with no activity.

@Aariq
Copy link
Collaborator

Aariq commented Jul 21, 2022

Not only is there code commented out in met2CF.csv.R, but there's not actually any valid testthat tests. Un-stale-ing

@Sweetdevil144
Copy link
Contributor

It's been too long of this being open. Maybe we can add more test cases like :

library(ncdf4)
# Mock data
in.path <- "modules/data.atmosphere/tests/testthat/data"
in.file <- "test.met2CF.csv.csv"
outfolder <- tempdir()
lat <- 42 + 47 / 60 + 30 / 6000
lon <- 76 + 7 / 60 + 20 / 6000

# Define start_date and end_date
start_date <- as.Date("2024-01-01")
end_date <- as.Date("2024-12-31")

test_that("met2CF.csv handles inputs correctly", {
       # Test that function runs without errors
       expect_silent(PEcAn.data.atmosphere::met2CF.csv(in.path, in.file, outfolder, start_date, end_date, format, lat, lon))

       # Test that output directory contains the expected file
       expect_true(file.exists(file.path(outfolder, paste0(gsub(".csv", "", in.file), ".nc"))))

       # Test that the output file is a valid netCDF file
       nc_file <- nc_open(file.path(outfolder, paste0(gsub(".csv", "", in.file), ".nc")))
       expect_true(!is.null(nc_file))
       nc_close(nc_file)
})

Diving more into this test file for now.

@Aariq
Copy link
Collaborator

Aariq commented Apr 5, 2024

Take a look at this article on "test hygiene" and consider using withr::with_tempfile() instead of tempdir(): https://testthat.r-lib.org/articles/test-fixtures.html#test-hygiene

@Sweetdevil144
Copy link
Contributor

Current test file : https://github.com/Sweetdevil144/pecan/blob/develop/modules/data.atmosphere/tests/testthat/test.met2CF.csv.R

I think the format required has been changed since last times. Can anyone suggest approach for this error ?

> test_file('modules/data.atmosphere/tests/testthat/test.met2CF.csv.R')

══ Testing test.met2CF.csv.R ═══════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 0 ]2024-05-07 15:46:45.777093 ERROR  [test.met2CF.csv.R#39: PEcAn.data.atmosphere::met2CF.csv] : 
   datetime column is not specified in format 
[ FAIL 1 | WARN 0 | SKIP 0 | PASS 0 ]

── Error (test.met2CF.csv.R:39:8): met2CF.csv function works correctly ─────────
Error in `PEcAn.data.atmosphere::met2CF.csv(in.path = in.path, in.prefix = in.file, 
    outfolder = outfolder, format = format, lat = lat, lon = lon, 
    start_date = start_date, end_date = end_date)`: object 'alldatetime' not found
Backtrace:
    ▆
 1. └─PEcAn.data.atmosphere::met2CF.csv(...) at test.met2CF.csv.R:39:8
 2.   └─lubridate::year(alldatetime)
[ FAIL 1 | WARN 0 | SKIP 0 | PASS 0 ]

@mdietze
Copy link
Member

mdietze commented May 7, 2024

@Sweetdevil144 I recommend running the function line-by-line to figure out the source of the error. Also, I agree that the format in the test appears out-of-date. As the function itself says, the format comes from query.format.vars so I'd check what variables that function returns, as well as the format variables used within met2CF.csv to figure out what needs updating. You're definitely missing time.row in the test, but may be missing others. Indeed, I'd recommend also turning the currently failing test into a new test itself that expects a fail, as the function is successfully returning the error message "datetime column is not specified in format". Arguably, the function should exit at that point with an error code that later code (and testthat) can detect. If older versions of the function didn't require this "new" (but probably many years old) column, it seems like a good thing to test for.

@Sweetdevil144
Copy link
Contributor

Thanks for suggestions @mdietze . That helped. I've been trying to get an estimate of what start_date and end_date to consider for this SPECIFIC test.
As a default setting, I've been using following values of start_date and end_date :

start_date <- "2000-01-01"
end_date <- "2000-12-31"

# OR

start_date <- as.Date("2004-01-01")
end_date <- as.Date("2004-12-31")

However, these specific timestamps give following errors :

── Error (test.met2CF.csv.R:43:8): met2CF.csv function works correctly ─────────
Error in `PEcAn.logger::logger.severe("Data does not contain output after start_date or before end_date")`: Data does not contain output after start_date or before end_date
Backtrace:
    ▆
 1. └─PEcAn.data.atmosphere::met2CF.csv(...) at test.met2CF.csv.R:43:8
 2.   └─PEcAn.logger::logger.severe("Data does not contain output after start_date or before end_date")

I also tried printing the min and max ranges but couldn't get it Printed. Seems like sone logging restrictions.

print(paste("Date range in data:", min(alldatetime), "to", max(alldatetime)))

Any Suggestions?

@Sweetdevil144
Copy link
Contributor

Sweetdevil144 commented May 30, 2024

One thing I can't get my head around is as follow:

  1. Format specified for datetime in csv file = "%m/%d/%y %H:%M"
  2. However, by default, we're trying to set it to "%Y%m%d%H%M". Check this out:
if (datetime_units == "") {
        datetime_units <- "%Y%m%d%H%M" #assume ISO convention
      }

Hence we get following values of alldatetime and datetime_units on logging :

 alldatetime is = NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA
   NA NA NA NA NA

   where datetime_units = %Y%m%d%H%M

Error in `PEcAn.logger::logger.severe("\n\nData does not contain output after start_date or before end_date.")`:

Data does not contain output after start_date or before end_date.

Is the format specified in test.met2CF.csv.csv wrong? Or should we refactor our default format as "%m/%d/%y %H:%M" in met2CF.csv.R

@mdietze
Copy link
Member

mdietze commented May 30, 2024

The default format has to stay ISO.

If you're passing in data with a different time format, which the function allows, and are getting an error that suggests that either the format information is being passed in wrong or there's a bug in how that format information is being read and applied.

@Sweetdevil144
Copy link
Contributor

Okay. So the issue was caused due to an improper format. Seems I made some blunders while setting formats. Suites Prepared !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants