search

CliMA / Thermodynamics.jl

A package containing a library of moist thermodynamic relations.
https://clima.github.io/Thermodynamics.jl/dev/
Apache License 2.0
61 stars 2 forks source link

Improve the explanation surrounding "virtual temperature" and a hypothetical "density temperature" #171

Closed glwagner closed 9 months ago

glwagner commented 10 months ago

I thought the description could be improved by being a little more explicit. If I've made a mistake, let me know and I can fix it. I also embellished the "side point" by putting it in a box to emphasize that this is additional information that isn't core to understanding the main thread.

glwagner commented 10 months ago

Any idea why tests fail? Something about a variable test_project_toml_formatting that's not defined. It seems unrelated to this PR.

charleskawczynski commented 10 months ago

Any idea why tests fail? Something about a variable test_project_toml_formatting that's not defined. It seems unrelated to this PR.

Yeah, it's unrelated, Aqua's test_project_toml_formatting was removed. I'll open a PR to fix this. Also, this PR LGTM.

charleskawczynski commented 10 months ago

Feel free to rebase and merge

glwagner commented 10 months ago

The need to express everything in temperatures...

😅

Worth noting, I was reading this section because (some) bulk formula compute heat flux using the air-sea difference in virtual temperature. Blame the fluxes...

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (025a07f) 93.02% compared to head (0d97c3e) 93.02%. Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #171 +/- ## ======================================= Coverage 93.02% 93.02% ======================================= Files 9 9 Lines 1133 1133 ======================================= Hits 1054 1054 Misses 79 79 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

glwagner commented 10 months ago

Note I also changed Aqua.type_piracy to Aqua.type_piracies via https://github.com/JuliaTesting/Aqua.jl/pull/230

charleskawczynski commented 10 months ago

Can we rebase / squash this PR?

glwagner commented 10 months ago

Yes. Not sure how to do that at this point, should I disable auto-merge?

charleskawczynski commented 10 months ago

Yes, you can disable and then re-enable auto merge. Not sure why it didn't merge 🤔

glwagner commented 9 months ago

Superceded by #174