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

bugfix: HD line should be printed before SQ #345

Closed
wants to merge 1 commit into from

Conversation

nh13
Copy link
Contributor

@nh13 nh13 commented Mar 7, 2022

Fixes a long standing bug, but made more apparent in #336. Thank @jmarshall for pointing this out.

I tested this by on my local machine three ways.

  1. Not specifying -H, so the default header line should be emitted at the start of the SAM:
./bwa mem phix.fasta phix.fastq 
[M::bwa_idx_load_from_disk] read 0 ALT contigs
@HD VN:1.5  SO:unsorted GO:query
@SQ SN:gi|9626372|ref|NC_001422.1|  LN:5386
...
  1. Specifying -H with @HD, so the user provided header line should be emitted at the start of the SAM:
./bwa mem -H '@HD  VN:1.4  SO:unsorted     GO:query'  phix.fasta phix.fastq 
[M::bwa_idx_load_from_disk] read 0 ALT contigs
@HD VN:1.4  SO:unsorted GO:query
@SQ SN:gi|9626372|ref|NC_001422.1|  LN:5386
...
  1. Specifying -H with @CO, so the default header line should be emitted at the start of the SAM, while the CO line should be after the SQ lines.
./bwa mem -H '@CO  ID:id' phix.fasta phix.fastq 
[M::bwa_idx_load_from_disk] read 0 ALT contigs
@HD	VN:1.5	SO:unsorted	GO:query
@SQ	SN:gi|9626372|ref|NC_001422.1|	LN:5386
@CO	ID:id

Comment on lines +428 to +430
if (n_HD == 0) {
err_printf("@HD\tVN:1.5\tSO:unsorted\tGO:query\n");
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be a one-liner in the style of this repo

Suggested change
if (n_HD == 0) {
err_printf("@HD\tVN:1.5\tSO:unsorted\tGO:query\n");
}
if (n_HD == 0) err_printf("@HD\tVN:1.5\tSO:unsorted\tGO:query\n");

@jmarshall
Copy link
Contributor

Superceded by #348, which completely refactors this function.

@nh13
Copy link
Contributor Author

nh13 commented Aug 31, 2022

Closing in favor of #348 (more flexible) and in the hopes that this increases the chances the latter is merged!

@nh13 nh13 closed this Aug 31, 2022
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.

2 participants