Skip to content

Consolidate methods and standardize units for calculating saturated vapor pressure deficit - #3597

Merged
dlebauer merged 7 commits into
PecanProject:developfrom
dlebauer:saturated_vapor_pressure
Aug 15, 2025
Merged

Consolidate methods and standardize units for calculating saturated vapor pressure deficit#3597
dlebauer merged 7 commits into
PecanProject:developfrom
dlebauer:saturated_vapor_pressure

Conversation

@dlebauer

@dlebauer dlebauer commented Aug 14, 2025

Copy link
Copy Markdown
Member

consolidate get.es, SatVapPres functions and a new method proposed in #3590 into a single saturated_vapor_pressure function with args

❓ is there a preferred method to use as default?

#' @param method c("Magnus","ClausiusClapeyron","GoffGratch")
#' @param temp_units input temperature units ("degC","K","degF"), default "degC"
#' @param out_units output pressure units ("kPa","hPa","Pa","mb"), default "kPa"

Review Time Estimate

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.

@infotroph infotroph left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs Roxygen run and I see one logger call that will never fire, but otherwise looks good

Comment thread modules/data.atmosphere/R/metutils.R Outdated
Comment on lines +139 to +142
PEcAn.logger::logger.severe(
method,
"for converting temperature to saturated vapor pressure not supported"
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FTR match.arg will handle this case for you by erroring on no match, so this logger call will never fire.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Cool, thanks!

@dlebauer
dlebauer added this pull request to the merge queue Aug 15, 2025
Merged via the queue into PecanProject:develop with commit 7e959cb Aug 15, 2025
16 of 25 checks passed
@dlebauer
dlebauer deleted the saturated_vapor_pressure branch August 15, 2025 22:29
infotroph pushed a commit to infotroph/pecan that referenced this pull request Aug 16, 2025
…essure

Quick update to PecanProject#3597: export and document t2es alias; add venv/ to .Rbuildignore
@dlebauer dlebauer added the ccmmf issues and pre related to the ccmmf project label Sep 16, 2025
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 modules tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants