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

Fix overwrite not working for ctapipe-apply-models #2287

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

maxnoe
Copy link
Member

@maxnoe maxnoe commented Mar 13, 2023

Because there was no alias defined for the HDF5Merger component the --overwrite option had no effect for ctapipe-apply-models.

@maxnoe maxnoe added this to the v0.18.1 milestone Mar 13, 2023
@Tobychev
Copy link
Contributor

@maxnoe I edited the description with that I guessed the fix was, if you confirm the guess is accurate I can approve.

@maxnoe
Copy link
Member Author

maxnoe commented Mar 13, 2023

@Tobychev thanks, yes!

@kosack
Copy link
Contributor

kosack commented Mar 13, 2023

Because there was no alias defined for the HDF5Merger component the --overwrite option had no effect

Doesn't this mean that there simply wasn't an --overwrite option at all in the Tool? if so, passing --overwrite on the command-line should have raised an exception due to an unknown alias, right?

@maxnoe
Copy link
Member Author

maxnoe commented Mar 13, 2023

Doesn't this mean that there simply wasn't an --overwrite option at all in the Tool? if so, passing --overwrite on the command-line should have raised an exception due to an unknown alias, right?

The option now lives on the Tool itself, it only needs to be overridden if a tool as a sub-component that also needs the option.

@maxnoe maxnoe merged commit c24f38a into main Mar 14, 2023
@maxnoe maxnoe deleted the apply_models_overwrite branch March 14, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants