search

MSGFPlus / msgfplus

MS-GF+ (aka MSGF+ or MSGFPlus) performs peptide identification by scoring MS/MS spectra against peptides derived from a protein sequence database.
Other
76 stars 36 forks source link

Refinements in Tests #33

Closed ypriverol closed 6 years ago

ypriverol commented 6 years ago

@FarmGeek4Life @alchemistmatt @sangtaekim We are using msfgplus to reprocess some datasets. For that reason, we would like to contribute a little bit to the tool (if you are interested in that). I have made some changes to the tests in order to be able to run some of them or skip the ones that can't reproduce anymore. If you have some interest in accepting contribution please review this PR but if not you can close it.

alchemistmatt commented 6 years ago

I like the changes you have made to the unit tests; they look great. I'm a bit concerned that you removed all of the Eclipse files. Admittedly, we use IntelliJ now, and not Eclipse, but the Eclipse files may potentially still be useful to somebody, and weren't hurting anything. Furthermore, you removed both .idea/MSGFPlus.iml (newer) and MSGF-Plus.iml (older); I would prefer to have left .idea/MSGFPlus.iml in place. One option would be for us to accept the PR as-is then add back .idea/MSGFPlus.iml

We'll discuss the options then reply here.

ypriverol commented 6 years ago

Thanks @alchemistmatt, for your quick reply. For compatibility issues, we normally don't make available the IntellIJ or Eclipse projects configuration because this can change over time. For that reason, we prefer to only share the pom (maven config), which is the configuration that enables the final build/test/compile, etc . If you check around most of the big projects do not share the .idea or .project settings from IDEs.

In my experience, exchange those files only cause more confusion and sometimes errors. Because I can easily override your version when I do a commit. Here, a really nice explanation https://stackoverflow.com/questions/31764319/should-i-commit-my-idea-folder

alchemistmatt commented 6 years ago

I understand that; I still like to have MSGFPlus.iml and a few of the other config files that don't have absolute paths or user specific options (e.g. I definitely don't want to add .idea/workspace.xml). This allows me to see how the library versions have changed over time.

Also, in commit b6aade8188fe6b78 you added back idea/uiDesigner.xml; I presume that was an accident?

ypriverol commented 6 years ago

Also, in commit b6aade8 you added back idea/uiDesigner.xml; I presume that was an accident?

Yes probably was added by accident. Feel free to restore the .iml files and remove them from the .gitignore.

alchemistmatt commented 6 years ago

I misinterpreted the order of the commits; .idea/uiDesigner.xml is in fact gone. Thanks again for these expanded unit tests.

ypriverol commented 6 years ago

It would be extremely great if we can have examples of other instruments. I added the iprg2013 study. But if you have examples from other instruments, it should be great. My next task is to natively export to mztab. ;). With that we will not need to convert anymore from mzidentml to tsv

FarmGeek4Life commented 6 years ago

How do you plan to add the mztab output? Will that be an automatic (or optional) output parallel to the mzid, or a post-processing "convert mzid to mztab"?

I don't care if it's an optional output parallel to the mzid or a post-processing "convert mzid to mztab", but I would prefer that it not be output by default.

ypriverol commented 6 years ago

The idea would be to use a (or optional) output parallel. If you are ok with that. We have already convert tools from mzidentml to mztab but they are slow in term of performance.

FarmGeek4Life commented 6 years ago

I am fine with the optional parallel output, with the user needing to specify a command-line parameter like '-mztab 1' for it to be output