run_tool default arguments

[nbiederbeck]

ctapipe/ctapipe/core/tool.py

Line 542 in a633e0f

def run_tool(tool: Tool, argv=None, cwd=None, raises=False):

We only use run_tool in test files:

$ rg run_tool -l
ctapipe/conftest.py
ctapipe/core/__init__.py
ctapipe/core/tool.py
ctapipe/instrument/conftest.py
ctapipe/tools/tests/test_process_ml.py
ctapipe/tools/tests/test_process.py
ctapipe/tools/tests/test_merge.py
ctapipe/tools/tests/test_train.py
ctapipe/tools/tests/test_tools.py
ctapipe/tools/tests/test_apply_models.py
ctapipe/core/tests/test_tool.py
ctapipe/core/tests/test_traits.py

I assume it is not used externally.

I have two points to discuss:

1. It happens that cwd is not set properly and tests clutter the working directory with (prov-)log-files etc. Maybe it should have no default argument, so it gets set correctly by the developers implementing the tests?
2. raises is by default False, which I think is the wrong choice. One can either check correctly for raised errors when writing the tests, or if assert run_tool([...]) != 0 is really needed, set raises=False. I think this is mainly a quality of life change for developers debugging while implementing tests.

What do you think?

[maxnoe]

Maybe it should have no default argument, so it gets set correctly by the developers implementing the tests?

Or the default should be a tmpdir

[maxnoe]

What do you think?

Agreed.

[nbiederbeck]

Or the default should be a tmpdir

We are using at least dl1_tmp_path and dl2_tmp_path so I would say one should set it accordingly to whatever gets tested.

Not sure though if different temporary paths are needed at all