Fix alignment filename assumptions by tsibley · Pull Request #1206 · nextstrain/augur (original) (raw)
requested a review from a team
For example, .fa is another common suffix for FASTA filenames, but using it in the input alignment filename here resulted in both the temporary output alignment filename and the log file name being the same as the input alignment filename. This bug meant the input alignment file was overwritten! (twice!) and tree building failed when using IQ-TREE with such a filename.
We should assume as little as possible about filenames and use pathlib for all manipulations. The use of str.replace() here was particularly naive as it wasn't even anchored to the end of the string.
The change here would read a bit cleaner if build_iqtree() used Path objects internally instead of using strings, but that's a larger change for another commit/time.
Reported by Jon Bråte in a discussion post.¹
¹ <https://discussion.nextstrain.org/t/is-this-the-expected-behaviour-of-augur-tree/1375>
…ommand
log_file was missed by "shell-quote all user-defined filenames" (1b28739).
substition_model comes straight from the user via --substitution-model.
tsibley deleted the trs/fix-alignment-filename-assumptions branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})