augur icon indicating copy to clipboard operation
augur copied to clipboard

Discussion: Should `augur mask` error if not a single change is made (no-op)?

Open corneliusroemer opened this issue 2 years ago • 0 comments

During our issue triage meeting, we couldn't agree on the desired behaviour for augur mask if no change is made: Should mask error, warn, or do nothing at all.

As a compromise, there there could be a flag to customize what happens when mask is a no-op: e.g. one could send --no-op=warn to not error by default.

This is related to issues #945 #946 that I encountered when adapting the monkeypox build to produce nextclade reference trees. I wanted to configure the workflow to not mask anything at all. I emptied the bed file, this caused an error #945. I then set mask beginning and mask end to 0, this again threw an error #946.

My view currently is that this is harmfully overprotecting users. There are good reasons why no-ops are helpful: if no-ops were throwing errors, configuring a workflow like ncov to not error if nothing is masked would be tedious. One would need an extra function to wrap translate so as not to error.

Others were of the opinion that always erroring was good as it would protect the user from inaccuracies, e.g. by accident a .bed file is wrongly parametrized so nothing gets masked. It was felt by some that it wouldn't make sense to call mask if one didn't actually mask anything (I think that argument is bad since it's not always clear from the config whether no masking would happen or not, at least there are situations where it's hard to know a priori whether masking will be a no-op).

If there was a consensus against my preferred solution to accept no-op and error on no-op, I would very much like at least a flag that allows to override that behaviour, e.g. setting -on-no-op=warn (or error, or ok). E.g. in augur export we now also allow skipping of validation and it's been a useful feature for me when working around bugs in other parts of the tool chain.

corneliusroemer avatar May 26 '22 14:05 corneliusroemer