Skip to content
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

Adding a plugin to calculate the virtual temperature #2058

Closed

Conversation

mo-philrelton
Copy link

Address: https://metoffice.atlassian.net/browse/EPPT-1965

This PR adds a plugin to calculate the virtual temperature based on the air temperature and humidity mixing ratio.

No CLI added due to the move to the DAG runner.

Testing:

  • Ran tests and they passed OK
  • Added new tests for the new feature(s)

CLA

  • If a new developer, signed up to CLA

Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.38%. Comparing base (84a8944) to head (1738285).
Report is 45 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2058      +/-   ##
==========================================
- Coverage   98.39%   98.38%   -0.01%     
==========================================
  Files         124      134      +10     
  Lines       12212    13083     +871     
==========================================
+ Hits        12016    12872     +856     
- Misses        196      211      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Katie-Howard Katie-Howard left a comment

Choose a reason for hiding this comment

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

I've looked through the changes and I'm generally happy with them:

  • The virtual temperature plugin looks like it does what it's supposed to
  • The unit tests that have written look sensible and all run fine
  • The PR is failing due to the PR standards - I think it's just something to do with adding yourself as a contributor. I think it's because in the email address you are [email protected] but you have put your name as Philip it doesn't like this. It might be worth changing that and see if it then passes that test?
  • I've left a couple of comments about your comments but they are very minor.



class VirtualTemperature(BasePlugin):
"""Calculates the virtual temperature from temperature and ."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to write humidity mixing ratio here too?

Copy link
Author

Choose a reason for hiding this comment

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

I did, but I have now changed this string to """Plugin class to handle virtual temperature calculations.""" to better answer your second comment.

@staticmethod
def get_virtual_temperature(temperature: Cube, humidity_mixing_ratio: Cube) -> Cube:
"""
Calculate the virtual temperature from temperature and humidity mixing ratio.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to repeat yourself here?

Copy link
Author

Choose a reason for hiding this comment

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

You're right it's a little repetitive. I have changed the class docstring.

@mo-philrelton
Copy link
Author

The cause of the failing tests was a poorly set up git config. Rather than confusing this PR with further commits and rewriting the history to the new configuration I will close this PR and open a new one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants