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

Minor point: preserve all injection times calculations in the object #43

Open
pavel-shliaha opened this issue Sep 1, 2017 · 2 comments

Comments

@pavel-shliaha
Copy link
Collaborator

Currently we only have

medianInjTime in the @coldata

the way we calculate which conditions to keep (for topN option) is we calculate the following numbers:

  1. medianInjTime (already in the colData)
  2. log2 ratio of injection time to medianInjTime
  3. rank of min(abs()) of number 2 above

could you please write all these numbers to the object not just 1. As a non-bioinformatician I prefer all these numbers to be stored and easily accessible.

BTW: This is true for all calculations, if we calculate smth I would perfer the calculations preserved as a separate column in the object (if the structure of the object permits this of course)

@sgibb
Copy link
Owner

sgibb commented Sep 1, 2017

I agree that we should add meaningful data to the colData slot. But

  1. log2 ratio of injection time to medianInjTime

is just log2(object$IonInjectionTimeMs / object$MedianIonInjectionTimeMs). So it is very easy to calculate (and very fast) and adding another column would increase the amount of memory the whole object needs. If the information is important or you really need it I will add it. But because it is so easy to calculate (and the user could manually add it via object$log2Ratio <- abs(log2(object$IonInjectionTimeMs / object$MedianIonInjectionTimeMs)) I am against adding it per default.

  1. rank of min(abs()) of number 2 above

There are a few problems with this number:

  1. To avoid multiple sorting I initially sort all ratios and do the split afterwards (you did it the other way around: first split than multiple rank calls; sorting is the speed bottleneck). That's why you would get strange numbers not starting at 1 for almost all ranks (but that could be changed of course).
  2. Because I calculate this numbers in the method filterIonInjectionTime and the user has no option to get the temporary object (with all ranks) all ranks in the filtered object would be 1:keepTopN (which would not very informative, especially for our default keepTopN=2).

@pavel-shliaha
Copy link
Collaborator Author

this is not a major issue now. Provided our filtering on inj times agrees, but in general I think we should try to actually add all this info to the object. I understand that this might require changing the code, so lets do it much later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants