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

Create a sequence of values with increasing differences between values #121

Merged
merged 4 commits into from
Jan 24, 2024

Conversation

jorainer
Copy link
Member

This PR adds a simple function breaks_ppm that calculates a sequence of values with increasing difference between pairs of values. This is in contrast to seq that creates a sequence of values with a constant difference between values.

A use case of this function would be e.g. for binning of peaks in mass spectra with bin sizes that increase with the m/z. The current default (that simply uses seq and thus a constant bin size) does not properly account for m/z dependent errors of (TOF) MS instruments.

I'm not super-happy with the name of the function (breaks_ppm), thus, happy for suggestions. Also, the function could also (easily) be implemented in C to improve performance.

- Add function `breaks_ppm` to calculate a sequence of values with increasing
  difference between value pairs.
Copy link
Member

@sgibb sgibb left a comment

Choose a reason for hiding this comment

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

Thanks, I have a small suggestion.

R/binning.R Outdated
#' ## difference between the values increases (by ppm)
#' diff(res)
breaks_ppm <- function(from = 1, to = 1, by = 1, ppm = 0) {
## Maybe implement in C for higher performance.
Copy link
Member

Choose a reason for hiding this comment

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

I would remove this comment. We can do it later if this function is really a bottle neck. But I assume it wouldn't be called very often.

R/binning.R Outdated Show resolved Hide resolved
Co-authored-by: Sebastian Gibb <[email protected]>
@jorainer jorainer requested a review from sgibb January 24, 2024 07:30
@sgibb sgibb merged commit 8d3e22e into main Jan 24, 2024
6 checks passed
@sgibb
Copy link
Member

sgibb commented Jan 24, 2024

@jorainer I already merged this PR but a version bump and NEWS.md entry are missing. Could you add these please?

@jorainer
Copy link
Member Author

Sure - hm, also strange that I did not include that...

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