Skip to content

Conversation

dlebauer
Copy link
Member

Add function that converts a generic events.json (see #3621 and #3551) into sipnet-specific events.in files.

Description

Includes:

  • write.events.SIPNET
  • documentation w/ example
  • tests

Motivation and Context

Required to use PEcAn to configure SIPNET to simulate agronomic managements.

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@dlebauer dlebauer added the ccmmf issues and pre related to the ccmmf project label Sep 16, 2025
Co-authored-by: Chris Black <[email protected]>
@dlebauer dlebauer requested a review from infotroph September 25, 2025 16:36
@infotroph
Copy link
Member

Merging dlebauer#8 should resolve the current test failures

@infotroph
Copy link
Member

  Error ('test-write.events.SIPNET.R:9:5'): write.events.SIPNET produces expected lines
  Error in `vapply(evs, function(e) e$date, character(1))`: values must be type 'character',
   but FUN(X[[1]]) result is type 'list'
  Backtrace:
   1. PEcAn.SIPNET::write.events.SIPNET(ev_json1, outdir)
        at test-write.events.SIPNET.R:9:5
   3. base::vapply(evs, function(e) e$date, character(1))

@dlebauer seems like the proximal issue is the entries in events_site1.json being list instead of scalar, but I don't understand the intention behind that format enough to recommend a fix.

@infotroph
Copy link
Member

@dlebauer reminder that this PR is still waiting for your input -- ping me if you want help on the test failures, but I'll need context from you to suggest the right fix.

@dlebauer
Copy link
Member Author

@infotroph if you want to take another look at the changes that I've made (f800475...dlebauer:write_events_sipnet) while trying to get the tests to pass, they include

  • move example events.json test fixtures to data.land package
  • create a validate_events_json function that validates an events.json against the schema
    • add jsonlite and jsonvalidator to data.land suggests [not sure if this should be Imports?]
    • add jsonvalidator to PEcAn.SIPNET suggests (not sure if this is necessary,
    • add tests
    • use this function in write.configs.SIPNET()
    • as implemented, if jsonvalidate is not installed, it skips the test and returns validate = TRUE. This was to avoid haveing jsonvalidate in imports for a single function. But perhaps that is where it belongs.

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

Labels

ccmmf issues and pre related to the ccmmf project Dockerfile Models Modules Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants