Only apply the cyclostrophic deprecation shim when the argument is passed

[dylanpulver]

compute_angular_windspeeds declares cyclostrophic: Optional[bool] = False but guards the deprecation shim with if cyclostrophic is not None:. Since False is not None, the shim fires on every call, including internal ones that never pass the argument, and then does model_kwargs["cyclostrophic"] = False — overwriting the caller's own setting with False. That makes cyclostrophic unreachable through model_kwargs, which is the exact route the deprecation message tells users to take.

Fixes #1209.

Changes

• cyclostrophic defaults to None instead of False, so the shim only runs when a caller actually passes it.
• model_kwargs is copied before the shim writes to it, so a caller's dict is never mutated.

Evidence, on develop, ER11 at r = 30 km (r_max = 40 km, v_max = 40 m/s):

call	v_ang (m/s)
_stat_er_2011(..., cyclostrophic=True)	38.400000
_stat_er_2011(..., cyclostrophic=False)	39.670883
compute_angular_windspeeds(..., model_kwargs={"cyclostrophic": True})	39.670883

The third row is the bug: the request is silently ignored. The caller's dict also comes back as {'cyclostrophic': False}.

No change to existing behaviour. Without the injected cyclostrophic=False each model falls back to its own signature default, which is what TropCyclone.from_tracks already documents. Numbers move only for callers who explicitly asked for cyclostrophic and were previously ignored.

Tests. Four tests in TestWindfieldHelpers, asserting values derived from Emanuel and Rotunno (2011) eq. (36) rather than from the code. The first three took the file 9 passed -> 12 and each fails against the unpatched source. The fourth (the warning is raised when the argument is passed) was added in review and is unexecuted here — CLIMADA's geospatial stack will not build in this environment, so CI is the check for it. Inputs and reference values are now defined once and shared with test_er_2011_pass; I parsed both revisions to confirm the shared fixture and expected array are identical to the literals that test used before, so it is unchanged by construction. black 24.4.2 and isort, the pinned versions, report no changes.

Two boxes left unticked deliberately: test_cross_antimeridian downloads the coast-distance grid from Zenodo (504 here; CI caches it), and I have not run the full suite for this revision.

Written with AI assistance; the wind speeds are function output and 38.4 m/s is hand-derived from the published equation.

PR Author Checklist

• Read the Contribution Guide
• Correct target branch selected (if unsure, select develop)
• Descriptive pull request title added
• Source branch up-to-date with target branch
• Documentation updated
• Tests updated
• Tests passing
• No new linter issues
• Changelog updated

PR Reviewer Checklist

• Read the Contribution Guide
• CLIMADA Reviewer Checklist passed
• Tests passing
• No new linter issues

[peanutfun]

@dylanpulver Thank you for your contribution! I would appreciate if you used your advertised agentic AI systems to shorten your PR description to only include relevant information. It took me some time to figure out what this PR was actually about.

While your changes are obvious and solid, the tests are not. They re-define data that is already present in other tests. Please make sure testing and reference data is only defined once, and then "shared" between tests that are using them. See my comments below.

[dylanpulver]

Thanks — all four addressed, and the description is rewritten to lead with what the PR is about rather than burying it.

• Shared fixture. The inputs and the reference values now live in one place (_er_2011_inputs() and ER11_EXPECTED) and test_er_2011_pass uses them too, so there is a single data source rather than a copy. ER11_NODE1_WITH_CORIOLIS is gone; it was row 1 of that same array.
• Trivial assertion. Removed, as suggested.
• Missing deprecation case. You read it right, and it was the more useful half. Added test_compute_angular_windspeeds_cyclostrophic_deprecation, which asserts the warning is raised when cyclostrophic is passed directly. The existing test only covered the absence of a spurious warning.
• Description. Cut roughly in half, leading with the defect.

One thing I should flag rather than let CI surface it: I could not run the suite for this revision — CLIMADA's geospatial stack does not build in my environment. So instead of asserting the refactor is safe, I parsed both revisions and compared: the shared fixture's literals and ER11_EXPECTED are identical to the ones test_er_2011_pass had inline (0 vs 0.0 being the only textual difference), so that test is unchanged by construction rather than by my reading of it. black 24.4.2 and isort with the profile from .pre-commit-config.yaml both report no changes. The new deprecation test is the one thing genuinely unverified locally.

[peanutfun]

@dylanpulver Thank you! I improved the tests to use the unittest fixture scheme and added an explicit test for the ER 2011 model with cyclostropic=True. Ready to merge.