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

Retrofit unit tests for fix_forecast_reference_time() #141

Open
9 tasks
truth-quark opened this issue Oct 9, 2024 · 1 comment
Open
9 tasks

Retrofit unit tests for fix_forecast_reference_time() #141

truth-quark opened this issue Oct 9, 2024 · 1 comment

Comments

@truth-quark
Copy link
Collaborator

truth-quark commented Oct 9, 2024

This issue follows on from issue #100 and PR #118.

PR #118 was reduced in scope to extracting the function & adding a very basic testing framework due to project handover requirements. This issue follows up to specify unit testing requirements:

Work on this PR needs to answer & address the following:

  • Is the time.units.calendar == 'gregorian' code branch & testing required?
  • Is the time.units.calendar == 'proleptic_gregorian' code branch & testing required?
  • What inputs should be tested?
  • What outputs should be tested?
  • What cube state/attrs should be tested?
  • Is the code branch covered by test_fix_forecast_reference_time_standard() required?
  • Should the hardcoded 1600 for ref date year be a configurable function arg?
  • @blimlim discussion on which branches to test
  • Does a better solution exist than patching PPField.calendar at runtime?

These questions will help determine what work is required.

@truth-quark
Copy link
Collaborator Author

Random Saturday night thought, modifying PPField.calendar is a hack as it's essentially monkeypatching in production:

# TODO: Is dynamically overwriting PPField acceptable?
PPField.calendar = pg_calendar

Being at the "root" level of the module, patching occurs at import time.

Question: would moving patching into a function help to:

  • Control when patching occurs
  • Provide unit testing benefits (e.g. make testing more deterministic as pre/post patching is controlled?)
  • Make patching optional (e.g. if the original functionality is required)

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

No branches or pull requests

1 participant