Add standard_result S3 class for unified preprocessing return values.

[shreyannandanwar]

Summary

Introduces standard_result() as a standardized S3 return object for preprocessing workflows.

This is an additive change and does not modify existing preprocessing behavior. The goal is to establish a common result contract that can be adopted incrementally by functions such as do_conversions(), met.process(), and related preprocessing utilities.

Changes

• Added standard_result() constructor
• Added preprocessing result validation helpers
• Added S3 print method
• Added as.data.frame() method
• Added comprehensive test coverage

Backward Compatibility

No existing workflow behavior has been modified.
No existing preprocessing functions have been migrated.
This PR only introduces the new abstraction and tests.

Testing

devtools::test("base/workflow", filter = "standard_result")

Result:

• 59 tests passed
• 0 failures

Full workflow test suite currently contains unrelated failures in run.write.configs_multisite, reproduced independently of this change.

Future Work

• Evaluate migration of met.process()
• Evaluate migration of convert_input()
• Evaluate migration of do_conversions()

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.

[S1DDHEY]

I'd suggest adding the @md tag somewhere in this Roxygen doc. Otherwise, the syntax highlighting may not be visible/render correctly.

[S1DDHEY]

should be done for other functions as well.

[S1DDHEY]

I'd suggest using PEcAn.logger::logger.severe() instead of these stop() for consistency with rest of the codebase

[S1DDHEY]

Nit: this absolute /Users/... path seems environment-specific. Not sure if this should be included or not.

[shreyannandanwar]

thanks, for the feedback
I will work on it.

[mdietze]

More generally, none of these paths should be hard coded. First, its unclear why a test would need to source any R file that's part of the package as the functions should be loaded already. Second, if you do need to load a file from within a package use system.file instead

[mdietze]

High-level comment that "standard_result" is an incredibly generic and non-descriptive name for any application. Its also a particularly bad choice for a class that processes inputs rather than outputs, as "result" and "output" are pretty much synonyms.

[mdietze]

I don't like the idea of hard coding the allowable tags within a low-level function. If the goal is to replace the BETY input type table, note that this table had been designed specifically to allow it to be extensible without recompiling any code. Burying it here means there's yet one more thing people need to remember to do when adding new models or expanding existing models. This specific list is also really incomplete and misleading (e.g., ED2 alone has 9 input tags: met, lu, thsum, veg, soil, pss, site, css, initcond; on the other side AFAIK no model has "fia" as in input tag)

[mdietze]

More generally, none of these paths should be hard coded. First, its unclear why a test would need to source any R file that's part of the package as the functions should be loaded already. Second, if you do need to load a file from within a package use system.file instead