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

Shift cyclostrophic argument to model_kwargs in from_tracks #936

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

spjuhel
Copy link
Collaborator

@spjuhel spjuhel commented Aug 9, 2024

Changes proposed in this PR:

  • Makes "cyclostrophic" a model_kwarg in from_tracks
  • Adds documentation on this possibility in from_tracks
  • Removes the explicit argument in compute_angular_windspeeds and _compute_windfields such that it uses the value in model_kwarg.
  • Changes LOGGER.debug to LOGGER.warning in _compute_angular_windspeeds_h10 to inform user that cyclostrophic=False is ignored in this case

This last point is subject to debate, raising an error is also possible. The question then is, should it be raised rather in the top-level (i.e., in from_tracks) or low-level (i.e., in _compute_angular_windspeeds_h10)

--

This PR fixes #897

PR Author Checklist

PR Reviewer Checklist

@spjuhel spjuhel changed the base branch from main to develop August 9, 2024 15:20
@spjuhel spjuhel self-assigned this Aug 9, 2024
@spjuhel spjuhel changed the title Feature/cyclostrophic as parameter Shift cyclostrophic argument to model_kwargs in from_tracks Aug 9, 2024
spjuhel pushed a commit to CLIMADA-project/climada_petals that referenced this pull request Aug 9, 2024
@spjuhel
Copy link
Collaborator Author

spjuhel commented Aug 9, 2024

This CLIMADA-project/climada_petals@cfba26b should fix the test error for climada petal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conventions Coding conventions or style good first issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make cyclostrophic a model parameter in the Holland windfield model interface
3 participants