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.

Sign up for GitHub

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

Deprecated parameter alerts #673

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Deprecated parameter alerts #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions tests/test_zppy_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
import pytest

from zppy.utils import (
DeprecatedParameterError,
ParameterGuessType,
ParameterNotProvidedError,
add_dependencies,
check_for_deprecated_parameters,
check_parameter_defined,
check_required_parameters,
define_or_guess,
Expand Down Expand Up @@ -486,6 +488,21 @@ def test_check_parameter_defined():
check_parameter_defined(c, "d")


def test_check_for_deprecated_parameters():
deprecated_parameters = [
# Removed in https://github.com/E3SM-Project/zppy/pull/650
"e3sm_to_cmip_environment_commands",
"ts_fmt",
# Removed in https://github.com/E3SM-Project/zppy/pull/654
"scratch",
"atmosphere_only",
"plot_names",
]
c = {"scratch": "xyz", "plot_names": "a,b,c"}
with pytest.raises(DeprecatedParameterError):
check_for_deprecated_parameters(c, deprecated_parameters)


def test_get_file_names():
bash, settings, status = get_file_names("script_dir", "prefix")
assert bash == "script_dir/prefix.bash"
Expand Down
8 changes: 8 additions & 0 deletions zppy/global_time_series.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from zppy.logger import _setup_custom_logger
from zppy.utils import (
add_dependencies,
check_for_deprecated_parameters,
check_status,
get_file_names,
get_tasks,
Expand All @@ -31,6 +32,13 @@ def global_time_series(config, script_dir, existing_bundles, job_ids_file):

# --- Generate and submit global_time_series scripts ---
for c in tasks:
deprecated_parameters = [
# Removed in https://github.com/E3SM-Project/zppy/pull/654
"atmosphere_only",
"plot_names",
]
check_for_deprecated_parameters(c, deprecated_parameters)

c["ts_num_years"] = int(c["ts_num_years"])
# Loop over year sets
year_sets = get_years(c["years"])
Expand Down
6 changes: 6 additions & 0 deletions zppy/tc_analysis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from zppy.bundle import handle_bundles
from zppy.utils import (
check_for_deprecated_parameters,
check_status,
get_file_names,
get_tasks,
Expand All @@ -27,6 +28,11 @@ def tc_analysis(config: ConfigObj, script_dir: str, existing_bundles, job_ids_fi

# --- Generate and submit <task-name> scripts ---
for c in tasks:
deprecated_parameters = [
# Removed in https://github.com/E3SM-Project/zppy/pull/654
"scratch",
]
check_for_deprecated_parameters(c, deprecated_parameters)

dependencies: List[str] = []

Expand Down
7 changes: 7 additions & 0 deletions zppy/ts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from zppy.bundle import handle_bundles
from zppy.utils import (
ParameterGuessType,
check_for_deprecated_parameters,
check_status,
define_or_guess,
get_file_names,
Expand Down Expand Up @@ -32,6 +33,12 @@ def ts(config: ConfigObj, script_dir: str, existing_bundles, job_ids_file):

# --- Generate and submit ts scripts ---
for c in tasks:
deprecated_parameters = [
# Removed in https://github.com/E3SM-Project/zppy/pull/650
"e3sm_to_cmip_environment_commands",
"ts_fmt",
]
check_for_deprecated_parameters(c, deprecated_parameters)
set_mapping_file(c)
set_grid(c)
set_component_and_prc_typ(c)
Expand Down
15 changes: 15 additions & 0 deletions zppy/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ class DependencySkipError(RuntimeError):
pass


class DeprecatedParameterError(RuntimeError):
pass


# Utitlities for this file ####################################################


Expand Down Expand Up @@ -323,6 +327,17 @@ def check_parameter_defined(c: Dict[str, Any], relevant_parameter: str) -> None:
raise ParameterNotProvidedError(relevant_parameter)


def check_for_deprecated_parameters(
c: Dict[str, Any], deprecated_parameters: List[str]
) -> None:
present_parameters = []
for parameter in deprecated_parameters:
if parameter in c.keys():
present_parameters.append(parameter)
if present_parameters:
raise DeprecatedParameterError(present_parameters)
Comment on lines +337 to +338
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could also just do a warning, but I figured an error is more likely to get the user's attention.

Copy link
Collaborator

Choose a reason for hiding this comment

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

does ts_fmt a no-op or raise error before this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

on main, it just won't get used by anything. That is, if a user sets it thinking it's useful, they'll get no indication (either warning or error) that it's not actually useful. (Confirmed with cfg in #677).



def get_file_names(script_dir: str, prefix: str):
return tuple(
[
Expand Down
Loading