topdownproteomics / sdk

Software solution for common top-down proteomics tasks
http://www.topdownproteomics.org/
MIT License
9 stars 4 forks source link

catch non-amino acid uppercase letters #50

Closed rmillikin closed 6 years ago

rfellers commented 6 years ago

Do we accept the J amino acid? I thought this came up and the answer was yes ... but I could be mistaken

rmillikin commented 6 years ago

actually yeah, the ProForma paper does list it as accepted (I or L)

rfellers commented 6 years ago

Nice, thanks for running this down to confirm! I'd like to see a unit test that passes when you send in U, O, B, J, and Z and new term comes out (i.e. no exception). That would round out this PR nicely and would keep something like this from happening in the future. Sorry if that sounds bossy ... I would do it myself, but it just seems to fit here with this PR.

acesnik commented 6 years ago

Could we also have a test that throws an informative exception if X is used? We specifically disallow that in the ProForma paper.

acesnik commented 6 years ago

Oh wait, Ryan already wrote that one in. Nice! https://github.com/topdownproteomics/sdk/pull/50/files#diff-83e6e0c4cc47cbb8397628985caebbd9R352