Skip to content

Conversation

@dcamron
Copy link
Member

@dcamron dcamron commented Dec 23, 2024

Taking recommendations from community input, these calculations introduce temperature and moisture dependence to relevant "constant" quantities for use in upcoming analytic saturation vapor pressure and LCL calculations.

Beyond code review, specifically requesting feedback on function names and useful tests I might've missed. Will start playing around with satvap and LCL shortly this holiday!

Checklist

  • Tests added
  • Fully documented

@dcamron dcamron added Type: Feature New functionality Area: Calc Pertains to calculations Subarea: Thermo Pertains to thermodynamic calculations and their physical correctness labels Dec 23, 2024
@dcamron dcamron requested a review from a team as a code owner December 23, 2024 20:34
@dcamron dcamron requested review from dopplershift and removed request for a team December 23, 2024 20:34
@dcamron
Copy link
Member Author

dcamron commented Dec 24, 2024

Currently attempting to massage these into the process_units pipeline, may will just include those changes here.

@dcamron dcamron removed the Status: Needs Review Pull request needs review label Dec 24, 2024
@dcamron dcamron force-pushed the not-constants branch 2 times, most recently from 5e1ac05 to b432ffe Compare December 24, 2024 19:14
@dcamron
Copy link
Member Author

dcamron commented Dec 24, 2024

@jthielen let me know your thoughts on using process_units in this way. I'm not sure if these new dimensions could cause current or future registry interoperability issues, but it seemed to be a relatively clean extension of your work.

Add necessary constants for new calculations. Fix weird malformed table errors with units column spaces.
dopplershift
dopplershift previously approved these changes Jan 6, 2025
@dopplershift dopplershift merged commit ea2fcfc into Unidata:main Jan 6, 2025
35 checks passed
@jthielen
Copy link
Collaborator

jthielen commented Jan 7, 2025

@jthielen let me know your thoughts on using process_units in this way. I'm not sure if these new dimensions could cause current or future registry interoperability issues, but it seemed to be a relatively clean extension of your work.

I'm very sorry for the delay in getting to this after it's already been approved and merged, but in case it matters for future reference, this usage of process_units is perfectly in line with how I envisioned it working.

@dcamron
Copy link
Member Author

dcamron commented Jan 7, 2025

No need to apologize, we can always open a new PR if internals need work. Glad to hear it! :)

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

Labels

Area: Calc Pertains to calculations Subarea: Thermo Pertains to thermodynamic calculations and their physical correctness Type: Feature New functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants