Skip to content

Commit

Permalink
Merge pull request #2666 from cta-observatory/fix/unknown_config_type
Browse files Browse the repository at this point in the history
Fixed unknown configs being silently ignored
  • Loading branch information
kosack authored Dec 4, 2024
2 parents 71ebd9e + 491c94d commit df2f90d
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 2 deletions.
6 changes: 6 additions & 0 deletions docs/changes/2666.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Fixed a bug where if a configuration file with unknown file extension was passed
to a tool, e.g. ``--config myconf.conf`` instead of ``--config myconf.yaml``, it
was silently ignored, despite an info log saying "Loading config file
myconf.conf". Configuration files must now have one of the following extensions
to be recognized: yml, yaml, toml, json, py. If not a ``ToolConfigurationError``
is raised.
31 changes: 31 additions & 0 deletions src/ctapipe/core/tests/test_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,3 +569,34 @@ def test_exit_status_interrupted(tmp_path, provenance):
provlog = activities[0]
assert provlog["activity_name"] == InterruptTestTool.name
assert provlog["status"] == "interrupted"


def test_no_ignore_bad_config_type(tmp_path: Path):
"""Check that if an unknown type of config file is given, an error is raised
instead of silently ignoring the file (which is the default for
traitlets.config)
"""

class SomeTool(Tool):
float_option = Float(1.0, help="An option to change").tag(config=True)

test_config_file = """
SomeTool:
float_option: 200.0
"""

bad_conf_path = tmp_path / "test.conf" # note named "conf" not yaml.
bad_conf_path.write_text(test_config_file)

good_conf_path = tmp_path / "test.yaml"
good_conf_path.write_text(test_config_file)

tool = SomeTool()

# here we should receive an error.
with pytest.raises(ToolConfigurationError):
tool.load_config_file(bad_conf_path)

# test correct case:
tool.load_config_file(good_conf_path)
assert tool.float_option > 1
11 changes: 9 additions & 2 deletions src/ctapipe/core/tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,16 @@ def load_config_file(self, path: str | pathlib.Path) -> None:
with open(path, "rb") as infile:
config = Config(toml.load(infile))
self.update_config(config)
else:
# fall back to traitlets.config.Application's implementation
elif path.suffix in [".json", ".py"]:
# fall back to traitlets.config.Application's implementation. Note
# that if we don't specify the file suffixes here, traitlets seems
# to silently ignore unknown ones.
super().load_config_file(str(path))
else:
raise ToolConfigurationError(
f"The config file '{path}' is not in a known format. "
"The file should end in one: yml, yaml, toml, json, py"
)

Provenance().add_input_file(path, role="Tool Configuration", add_meta=False)

Expand Down

0 comments on commit df2f90d

Please sign in to comment.