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

Added description to prepareExtractionCoordinates_ function, need review #2

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

shubham1637
Copy link

I added some description to the prepareExtractionCoordinates_ function. I am not sure about the transition_exp_used size.

@shubham1637 shubham1637 requested a review from hroest March 30, 2018 22:07
// chrom_list contains list of pointers for empty chromatograms. coordinates is also an empty vector of coordinates for each chromatogram.
// transition_exp_used contains peptide name, protein name. Is transition_exp_used one transition or many?
// trafo_inverse has mapping from iRT to seconds. cp has further params for extracting chromatogram.
// For example, function will fill one coordinate as : 400 Th, 785 Th, (1800 + 300)s, (1800 -300)s, id.
Copy link

Choose a reason for hiding this comment

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

I think this should go into the header

@hroest
Copy link

hroest commented Apr 2, 2018

I suggest to actually annotate functions in the header since this then also shows up in the documentation, also we already have some documentation there so you may want to merely enhance it:

/** @brief Function to prepare extraction coordinates that also correctly handles RT transformations
*
* Creates a set of (empty) chromatograms and extraction coordinates with
* the correct ids, m/z and retention time start/end points to be extracted
* by the ChromatogramExtractor.
*
* Handles rt extraction windows by calculating the correct transformation
* for each coordinate.
*
* @param chrom_list Output of chromatograms (will be filled with empty chromatogram ptrs)
* @param coordinates Output of extraction coordinates (will be filled with matching extraction coordinates)
* @param transition_exp_used The transition experiment used to create the coordinates
* @param ms1 Whether to perform MS1 (precursor ion) or MS2 (fragment ion) extraction
* @param trafo_inverse Inverse transformation function
* @param cp Parameter set for the chromatogram extraction
*
*/
void prepareExtractionCoordinates_(std::vector< OpenSwath::ChromatogramPtr > & chrom_list,
std::vector< ChromatogramExtractorAlgorithm::ExtractionCoordinates > & coordinates,
const OpenSwath::LightTargetedExperiment & transition_exp_used,
const bool ms1, const TransformationDescription trafo_inverse,
const ChromExtractParams & cp) const;

@hroest hroest closed this Apr 2, 2018
@hroest hroest reopened this Apr 2, 2018
@@ -908,8 +913,8 @@ namespace OpenMS
// Then correct the start/end positions and add the extra_rt_extract parameter
prepare_coordinates_sub(chrom_list, coordinates, transition_exp_used, 0.0, ms1);
for (std::vector< ChromatogramExtractor::ExtractionCoordinates >::iterator it = coordinates.begin(); it != coordinates.end(); ++it)
{
it->rt_start = trafo_inverse.apply(it->rt_start) - (cp.rt_extraction_window + cp.extra_rt_extract)/ 2.0;
{// iterator it parse through complete vector of coordinates and all transitions have RT end/start set to +/-300 seconds.
Copy link

Choose a reason for hiding this comment

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

probably we dont want to write explicit numbers in here as 300 seconds is just an example value, not a fixed value

hroest pushed a commit that referenced this pull request Apr 2, 2018
hroest pushed a commit that referenced this pull request Nov 12, 2021
hroest pushed a commit that referenced this pull request Nov 12, 2021
hroest pushed a commit that referenced this pull request Mar 1, 2022
hroest pushed a commit that referenced this pull request Mar 1, 2022
jcharkow added a commit that referenced this pull request Jun 9, 2022
* read swath window file with no header

In response to OpenMS#6055 allow
openSwath to be able to read window files with no header.

* fix lint

* Revert "fix lint"

This reverts commit 83875b6.

* Apply suggestions from code review

* change std::cout to OPENMS_LOG_INFO

* In header check, check for "e" float nums

previous check would not detect examples like 4e2 as a valid swath
window (and would put this as a header instead). Allow "e" characters to
be supported.

Also revert back to std::cout because OPENMS_LOG_INFO does not seem to
be working.

* minor style edits

* replace std:cerr and std::cout with OPENMS LOG

when use std::endl instead of \n the OPENMS LOG statements work

* accept tab deliminators

allow for headers separated by tab deliminators to also be
accepted

Co-authored-by: GitHub Linter <[email protected]>
Co-authored-by: Timo Sachsenberg <[email protected]>
jcharkow pushed a commit that referenced this pull request Jan 17, 2023
[FIX] Add new DBsearchDeconvMass util to list of executables to be built (and documented)
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