marcelm
commented
1 year ago You could say that the tests document existing, expected TREX behavior. If a change breaks a test, it means that backwards compatibility is lost, and I would say we need to discuss this individually (per change) whether that is ok or not.
Currently, since we don’t have that many users, I would in general tend more towards just making the change and breaking backwards compatibility if the change is obviously beneficial.
Since initially you had worked on the program by yourself in your own repository, it was a good choice to preserve backwards compatibility as much as possible by making new behavior optional, but adding flags also has a cost: For example, it increases the number of code paths that need to be tested, the flag needs to be explained and there’s a chance that users forget to enable it.
For low-complexity cloneIDs, I would assume that it is always better to filter them out, so I would say that perhaps this doesn’t need a flag. I realize this comment comes to late to change anything about #36, so maybe this could be done in a followup PR. In any case, this needs Leonie’s approval (in the PR making the behavior the default or in a separate issue).
In any case, I just added a changelog to the repository (CHANGES.md) and we should document the changes we make there.
One of the current tests for TREX is assessing that CloneIDs from previous experiments are read and recovered in the same way. I was trying to add a new function to remove CloneIDs with a single base or a single repeated base. This makes the test fail. Should we change the test or add the option for new versions to only be active if a flag is passed?