Skip to content

Feature request: peptide position information in results#223

Open
mansanlab wants to merge 1 commit intolazear:masterfrom
mansanlab:master
Open

Feature request: peptide position information in results#223
mansanlab wants to merge 1 commit intolazear:masterfrom
mansanlab:master

Conversation

@mansanlab
Copy link
Copy Markdown

Fixes #110

Copy link
Copy Markdown
Owner

@lazear lazear left a comment

Choose a reason for hiding this comment

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

Tbh I don't really like the idea of adding these columns:

  • Unclear how or if this interacts with new protein grouping module
  • Unclear what the value is, especially considering that:
  • These are trivial to compute by the end user

vec![
"psm_id",
"peptide",
"first_aa",
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

"first_aa" and "last_aa" seem tautological, and trivially computable for end users

start: Instant,
}

fn feature_headers() -> Vec<&'static str> {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What's the rationale for lifting these to functions?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Had to fight the borrow-checker less this way, but I know there are simpler ways to do this.

}

#[cfg(test)]
mod test {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I am generally in favor of tests, but these seem unnecessary

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fair enough, will remove

required byte_array scannr (utf8);
required byte_array peptide (utf8);
required byte_array stripped_peptide (utf8);
required byte_array first_aa (utf8);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

see above

}

#[cfg(test)]
mod test {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

see above

@mansanlab
Copy link
Copy Markdown
Author

The value personally comes entirely from next_aa, prev_aa, starts, and ends; all others come for "free". I included them to be pedantic and it would be easier to remove than to add them later should you (or others) find these useful.

I use next_aa, prev_aa, starts, and ends for mapping semi-tryptic peptide sites to structure and to determine the semi-tryptic ended-ness; N-term vs C-term semi-enzymatic cleavage.

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.

Feature request: peptide position information in results

2 participants