Skip to content

chore: upstream cigar#57

Open
flying-sheep wants to merge 4 commits into
mainfrom
pa/cigar-to-st
Open

chore: upstream cigar#57
flying-sheep wants to merge 4 commits into
mainfrom
pa/cigar-to-st

Conversation

@flying-sheep
Copy link
Copy Markdown
Member

@flying-sheep flying-sheep commented May 15, 2026

TODO: why are we rolling our own CigarOp instead of using noodles::sam::alignment::record::cigar::Op?

The only difference seems to be that that one has a Pad variant that we don’t have.

@flying-sheep flying-sheep requested a review from Psy-Fer May 15, 2026 07:38
@Psy-Fer
Copy link
Copy Markdown
Collaborator

Psy-Fer commented May 15, 2026

Oh yea so funny story.
Claude absolutely has no idea how to deal with cigar strings and messed this up so many times, that I wrote a quick version myself to get it on track. Forgot to go back and fix it up. 😅

@flying-sheep
Copy link
Copy Markdown
Member Author

So you’re saying that we should get rid of CigarOp and use the upstream version?

@Psy-Fer
Copy link
Copy Markdown
Collaborator

Psy-Fer commented May 15, 2026

I'm saying we should fix it.

@flying-sheep
Copy link
Copy Markdown
Member Author

flying-sheep commented May 15, 2026

Is that a yes? Otherwise I don’t know what “fix” means, as I don’t know of anything that’s broken, just duplicate code that we can eliminate.

@flying-sheep flying-sheep changed the title chore: unify cigar stringification chore: upstream cigar May 15, 2026
Comment thread src/align/read_align.rs
a.genome_end,
a.is_reverse,
&a.cigar,
//&a.cigar,
Copy link
Copy Markdown
Member Author

@flying-sheep flying-sheep May 15, 2026

Choose a reason for hiding this comment

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

This (and another cmp farther down) are the only casualties: why is this here? Can a vector of cigar ops be “greater” or “less” than another? (other than length)

Comment thread src/align/read_align.rs
genome_pos = acceptor;
}
CigarOp::Ins(_) | CigarOp::SoftClip(_) | CigarOp::HardClip(_) => {}
Kind::Insertion | Kind::SoftClip | Kind::HardClip | Kind::Pad => {}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added Pad to all the “else” branches. Is that correct?

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