Skip to content

Commit

Permalink
Pylint fixups (#886)
Browse files Browse the repository at this point in the history
# Pull Request

## Title

Additional fixups for the most recent versions of python/pylint.

---

## Description

- Converts some quote inconsistency ignores to `"""` strings
- Ignores a `Generator` typehint issue that changed with 3.13
- Adjusts a comment in pyproject.toml to help us track what needs to
change after dropping support for 3.8 - #749.

---

## Type of Change

- 🔄 Refactor

---
  • Loading branch information
bpkroth authored Nov 25, 2024
1 parent 022cae8 commit 8de0a2f
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 14 deletions.
2 changes: 1 addition & 1 deletion mlos_bench/mlos_bench/optimizers/base_optimizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def _validate_json_config(self, config: dict) -> None:

def __repr__(self) -> str:
opt_targets = ",".join(
f"{opt_target}:{({1: 'min', -1: 'max'}[opt_dir])}"
f"""{opt_target}:{({1: "min", -1: "max"}[opt_dir])}"""
for (opt_target, opt_dir) in self._opt_targets.items()
)
return f"{self.name}({opt_targets},config={self._config})"
Expand Down
2 changes: 1 addition & 1 deletion mlos_bench/mlos_bench/optimizers/mlos_core_optimizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def __init__(
if "max_trials" not in self._config:
self._config["max_trials"] = self._max_suggestions
assert int(self._config["max_trials"]) >= self._max_suggestions, (
f"max_trials {self._config.get('max_trials')} "
f"""max_trials {self._config.get("max_trials")} """
f"<= max_suggestions{self._max_suggestions}"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def _set_default_params(self, params: dict) -> dict: # pylint: disable=no-self-
# since this is a common way to set the deploymentName and can same some
# config work for the caller.
if "vnetName" in params and "deploymentName" not in params:
params["deploymentName"] = f"{params['vnetName']}-deployment"
params["deploymentName"] = f"""{params["vnetName"]}-deployment"""
_LOG.info(
"deploymentName missing from params. Defaulting to '%s'.",
params["deploymentName"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ def _set_default_params(self, params: dict) -> dict: # pylint: disable=no-self-
# since this is a common way to set the deploymentName and can same some
# config work for the caller.
if "vmName" in params and "deploymentName" not in params:
params["deploymentName"] = f"{params['vmName']}-deployment"
params["deploymentName"] = f"""{params["vmName"]}-deployment"""

_LOG.info(
"deploymentName missing from params. Defaulting to '%s'.",
params["deploymentName"],
Expand Down Expand Up @@ -239,7 +240,7 @@ def wait_host_operation(self, params: dict) -> Tuple[Status, dict]:
"""
_LOG.info("Wait for operation on VM %s", params["vmName"])
# Try and provide a semi sane default for the deploymentName
params.setdefault(f"{params['vmName']}-deployment")
params.setdefault(f"""{params["vmName"]}-deployment""")
return self._wait_while(self._check_operation_status, Status.RUNNING, params)

def wait_remote_exec_operation(self, params: dict) -> Tuple["Status", dict]:
Expand Down
4 changes: 2 additions & 2 deletions mlos_bench/mlos_bench/services/remote/ssh/ssh_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ def id_from_connection(connection: SSHClientConnection) -> str:
def id_from_params(connect_params: dict) -> str:
"""Gets a unique id repr for the connection."""
return (
f"{connect_params.get('username')}@{connect_params['host']}"
f":{connect_params.get('port')}"
f"""{connect_params.get("username")}@{connect_params["host"]}"""
f""":{connect_params.get("port")}"""
)

def connection_made(self, conn: SSHClientConnection) -> None:
Expand Down
4 changes: 2 additions & 2 deletions mlos_bench/mlos_bench/storage/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ def kv_df_to_dict(dataframe: pandas.DataFrame) -> Dict[str, Optional[TunableValu
data = {}
for _, row in dataframe.astype("O").iterrows():
if not isinstance(row["value"], TunableValueTypeTuple):
raise TypeError(f"Invalid column type: {type(row['value'])} value: {row['value']}")
raise TypeError(f"""Invalid column type: {type(row["value"])} value: {row["value"]}""")
assert isinstance(row["parameter"], str)
if row["parameter"] in data:
raise ValueError(f"Duplicate parameter '{row['parameter']}' in dataframe")
raise ValueError(f"""Duplicate parameter '{row["parameter"]}' in dataframe""")
data[row["parameter"]] = (
try_parse_val(row["value"]) if isinstance(row["value"], str) else row["value"]
)
Expand Down
8 changes: 4 additions & 4 deletions mlos_bench/mlos_bench/tests/dict_templater_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def test_os_env_expansion(source_template_dict: Dict[str, Any]) -> None:

results = DictTemplater(source_template_dict).expand_vars(use_os_env=True)
assert results == {
"extra_str-ref": f"{environ['extra_str']}-ref", # pylint: disable=inconsistent-quotes
"extra_str-ref": f"""{environ["extra_str"]}-ref""",
"str": "string",
"str_ref": "string-ref",
"secondary_expansion": "string-ref",
Expand All @@ -103,7 +103,7 @@ def test_os_env_expansion(source_template_dict: Dict[str, Any]) -> None:
],
"dict": {
"nested-str-ref": "nested-string-ref",
"nested-extra-str-ref": f"nested-{environ['extra_str']}-ref",
"nested-extra-str-ref": f"""nested-{environ["extra_str"]}-ref""",
},
}

Expand All @@ -116,7 +116,7 @@ def test_from_extras_expansion(source_template_dict: Dict[str, Any]) -> None:
}
results = DictTemplater(source_template_dict).expand_vars(extra_source_dict=extra_source_dict)
assert results == {
"extra_str-ref": f"{extra_source_dict['extra_str']}-ref", # pylint: disable=inconsistent-quotes # noqa: E501
"extra_str-ref": f"""{extra_source_dict["extra_str"]}-ref""",
"str": "string",
"str_ref": "string-ref",
"secondary_expansion": "string-ref",
Expand All @@ -134,6 +134,6 @@ def test_from_extras_expansion(source_template_dict: Dict[str, Any]) -> None:
],
"dict": {
"nested-str-ref": "nested-string-ref",
"nested-extra-str-ref": f"nested-{extra_source_dict['extra_str']}-ref", # pylint: disable=inconsistent-quotes # noqa: E501
"nested-extra-str-ref": f"""nested-{extra_source_dict["extra_str"]}-ref""",
},
}
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,11 @@ disable = [
"fixme",
"no-else-return",
"consider-using-assignment-expr",
"deprecated-typing-alias", # disable for now - only deprecated recently
"deprecated-typing-alias", # disable for now - still supporting python 3.8
"docstring-first-line-empty",
"consider-alternative-union-syntax", # disable for now - still supporting python 3.8
"missing-raises-doc",
"unnecessary-default-type-args", # affects Generator type hints, but we still support python 3.8
]

[tool.pylint.string]
Expand Down

0 comments on commit 8de0a2f

Please sign in to comment.