-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve the explanation surrounding "virtual temperature" and a hypothetical "density temperature" #171
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
The need to express everything in temperatures...
Any idea why tests fail? Something about a variable |
Yeah, it's unrelated, Aqua's |
Fix CI due to Aqua changes
Feel free to rebase and merge |
😅 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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #171 +/- ##
=======================================
Coverage 93.02% 93.02%
=======================================
Files 9 9
Lines 1133 1133
=======================================
Hits 1054 1054
Misses 79 79 ☔ View full report in Codecov by Sentry. |
Note I also changed |
Can we rebase / squash this PR? |
Yes. Not sure how to do that at this point, should I disable auto-merge? |
Yes, you can disable and then re-enable auto merge. Not sure why it didn't merge 🤔 |
Superceded by #174 |
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.