Skip to content

Mobt 1138 ignore git hash cli#2388

Merged
gavinevans merged 7 commits into
metoppv:masterfrom
mo-jbeaver:mobt_1138_ignore_git_hash_cli
May 28, 2026
Merged

Mobt 1138 ignore git hash cli#2388
gavinevans merged 7 commits into
metoppv:masterfrom
mo-jbeaver:mobt_1138_ignore_git_hash_cli

Conversation

@mo-jbeaver
Copy link
Copy Markdown
Contributor

@mo-jbeaver mo-jbeaver commented May 21, 2026

Related to https://github.com/metoppv/mo-blue-team/issues/1138, #2360
Acceptance test data: metoppv/improver_test_data#137

Builds on the work from PR: #2387
That PR exposes the option to ignore the grid hash within the spot extract CLI. The option to ignore the grid hash within the SpotExtract plugin was added in #2360.

This PR, adds a unit test covering the new variable added.

Testing:

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

Copy link
Copy Markdown

@mo-kbogue mo-kbogue left a comment

Choose a reason for hiding this comment

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

No major changes needed, just something in the docstrings, and maybe a renaming of 'ignore_grid_match'

Comment thread improver/cli/spot_extract.py Outdated
suppress_warnings: bool = False,
realization_collapse: bool = False,
subset_coord: str = None,
ignore_grid_match: bool = False,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
ignore_grid_match: bool = False,
ignore_grid_match_check: bool = False,

Just one option, this could also be called something like ignore_grid_mismatch. The current variable name could be misleading. Should be changed throughout CLI and plugins.

suppress_warnings=suppress_warnings,
realization_collapse=realization_collapse,
subset_coord=subset_coord,
ignore_grid_match=ignore_grid_match,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
ignore_grid_match=ignore_grid_match,
ignore_grid_match_check=ignore_grid_match_check,

self,
neighbour_selection_method: str = "nearest",
fixed_lapse_rate: float = None,
ignore_grid_match: bool = False,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
ignore_grid_match: bool = False,
ignore_grid_match_check: bool = False,

Comment thread improver/spotdata/apply_lapse_rate.py Outdated
plugin = SpotLapseRateAdjust(
neighbour_selection_method=self.neighbour_selection_method
neighbour_selection_method=self.neighbour_selection_method,
ignore_grid_match=self.ignore_grid_match,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
ignore_grid_match=self.ignore_grid_match,
ignore_grid_match_check=self.ignore_grid_match_check,

plugin = SpotLapseRateAdjust(
neighbour_selection_method=self.neighbour_selection_method,
fixed_lapse_rate=self.fixed_lapse_rate,
ignore_grid_match=self.ignore_grid_match,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
ignore_grid_match=self.ignore_grid_match,
ignore_grid_match_check=self.ignore_grid_match_check,

Comment thread improver_tests/spotdata/test_SpotManipulation.py Outdated
Comment thread improver_tests/spotdata/test_SpotManipulation.py Outdated
"neighbour_data", [np.array([[[0, 1, 2], [0, 1, 2], [0, 1, 2]]])]
)
def test_ignore_grid_match_argument(neighbour_cube, apply_lapse_rate_correction):
"""Test that when the ignore_grid_match argument is set to True, that the function
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
"""Test that when the ignore_grid_match argument is set to True, that the function
"""Test that when the ignore_grid_match_check argument is set to True, that the function

@mo-jbeaver
Copy link
Copy Markdown
Contributor Author

The newest commit updates the docstrings and comments made during the review.
I believe the variable name ignore_grid_match came from elsewhere so have chosen to keep it as it is for the time being.

Copy link
Copy Markdown
Contributor

@brhooper brhooper left a comment

Choose a reason for hiding this comment

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

Thanks @mo-jbeaver, I've just added a couple of comments suggesting some additional tests of the new option.

Comment thread improver/spotdata/apply_lapse_rate.py
Comment thread improver/cli/spot_extract.py
Copy link
Copy Markdown
Contributor

@Kat-90 Kat-90 left a comment

Choose a reason for hiding this comment

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

Ran the tests and checked the code. Happy to approve

Copy link
Copy Markdown
Contributor

@brhooper brhooper left a comment

Choose a reason for hiding this comment

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

Thanks @mo-jbeaver, the additional tests are helpful.

@gavinevans gavinevans merged commit 06ae487 into metoppv:master May 28, 2026
7 checks passed
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.

5 participants