`cnv_discordant_read_calls` has a `contig` param but supports contigs

[leehart]

All other functions that have a contig param use base_params.contig.

cnv_discordant_read_calls is the only function that uses base_params.contigs.

I suggest we rename the param to contigs and update internal usage accordingly, but this would mean a breaking change.

This could be done while fixing #660

[alimanfoo]

Hi @leehart, FWIW I'd be inclined to leave this as "contig". We haven't been very consistent about plural versus singular parameter names in general, but we do have lots of functions which have a "region" parameter that supports either a single or multiple regions, so it seems OK to have a similar situation for "contig". Doesn't seem worth an breaking API change.

[leehart]

Hi @alimanfoo, on reflection it seems a bit of a shame to keep shying away from making these kinds of improvements, as minor as they might be, out of fear of (or consideration for) some measure of inconvenience.

No doubt, we should try to avoid this situation, where the interface would be more self-consistent and make more sense if we slightly changed the name of a parameter here or there, if we can.

Personally, I'm in favour of making things better at the slight expense of making a major version incrementation. Otherwise we seem constantly trapped and held hostage by our past oversights or lack of foresight, stuck with inconsistencies and historical oddities.

I suppose, on balance, the question is: who does this approach really help? I can't imagine how new users would appreciate us holding back on these improvements. I'm conscious that existing users would have to update their code whenever they upgrade to a new major version, only very slightly in this case (from contig to contigs).

There is also the related issue of #375, where we intend to replace contig parameters with region instead, which will no doubt require a major version incrementation. We've needed to "break" the API at least 14 times now, and I expect we'll continue to do so, despite our best efforts and intentions. It's good to minimise the inconvenience. No one wants to force a change, but I'm thinking we shouldn't let the overall quality of the API suffer, by normalising and failing to resolve quirks like these, as long as we're moving in the right direction.

[leehart]

Relatedly, we could also move towards consistently naming function parameters that support multiple regions as regions instead of the singular region.

[leehart]

We could use a deprecation strategy, i.e. support both parameters for a time, to help give users a smoother transition towards the "correct" one, e.g. #703 (comment)