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.

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

Error message for non-existing config-file is suboptimal #1881

Closed
maxnoe opened this issue Apr 19, 2022 · 6 comments · Fixed by #2591
Closed

Error message for non-existing config-file is suboptimal #1881

maxnoe opened this issue Apr 19, 2022 · 6 comments · Fixed by #2591
Labels
Milestone

Comments

@maxnoe
Copy link
Member

maxnoe commented Apr 19, 2022

Describe the bug
Due to the new support for multiple config files, the error message for non-existing files is a bit verbose / confusing:

❯  ctapipe-process --config this/does/not/exist
2022-04-19 11:08:18,094 CRITICAL [ctapipe.ctapipe-process] (application.inner): Bad config encountered during initialization: The 'config_files' trait of a ProcessorTool instance contains  which expected <traitlets.traitlets.List object at 0x7f395f9dcd30>, not the str 'Path "/home/maxnoe/this/does/not/exist" does not exist'.

Expected behavior

Clear error message like configuration file "<file>" does not exist

@maxnoe maxnoe added the bug label Apr 19, 2022
@kosack
Copy link
Contributor

kosack commented Apr 19, 2022

I guess this is because the config_files Path trait fails first, as it checks the existence before any config loading happens. We could make a sub-class called ConfigPath perhaps that outputs a more useful error message?

@kosack
Copy link
Contributor

kosack commented Apr 19, 2022

I guess it's also the fact that this is a List(Path) trait, not just Path - not sure why it complains that it is not a List though, since that should not be a problem as it should accept it as a single-item list.

@maxnoe
Copy link
Member Author

maxnoe commented Mar 8, 2023

Just ran into this again, really, really misleading and annoying. Need to fix

@maxnoe maxnoe added this to the v0.18.1 milestone Mar 8, 2023
@maxnoe
Copy link
Member Author

maxnoe commented Mar 8, 2023

from ctapipe.core import Tool, run_tool


class TestTool(Tool):

    def setup(self):
        pass

    def start(self):
        print(self.config_files)



if __name__ == '__main__':
    run_tool(TestTool())

results in:

❯ python test_tool_config.py -c i_do_not_exist.yaml
[]

no error, just an empty config.

@maxnoe
Copy link
Member Author

maxnoe commented Mar 8, 2023

I have a hard time debugging this, since it never actually seems to enter ctapipe code, the values seem to be lost before in traitlets (e.g. Path.validate is never called on the given path)

@kosack
Copy link
Contributor

kosack commented Mar 8, 2023

once again #1406 ....

@maxnoe maxnoe modified the milestones: v0.18.1, v0.19.0 Mar 16, 2023
@kosack kosack modified the milestones: v0.20.0, v0.21.0 Sep 8, 2023
@maxnoe maxnoe modified the milestones: v0.21.0, 0.22.0 Mar 22, 2024
@maxnoe maxnoe modified the milestones: 0.22.0, 0.23.0 Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants