PNNL-m-q / mzapy

A Python package that provides an interface to raw MS data in the MZA format.
BSD 2-Clause "Simplified" License
7 stars 1 forks source link

peak fitting functions ought to return peak parameters as tuples for easier unpacking #21

Open dylanhross opened 9 months ago

dylanhross commented 9 months ago

I want to be able to do something like

peaks = find_peaks_1d_gauss(x_data, y_data, *other_params)
for peak_x, peak_ht, peak_wt in peaks:
    # iterate one peak at a time
    # and do stuff with the peak parameters
    ...

but instead it just returns separate arrays for each of the fitted parameters (x, ht, wt) so that you have to use zip to iterate peak by peak. I feel that iterating peak by peak (without zip, as in the example above) is a much more rational use case so these functions should return a list split by peaks rather than lists split by peak parameters.

Also a possible improvement would be to make them into generators that yield one peak at a time.

dylanhross commented 2 weeks ago

Something else to think about: I already have an amount of code that uses these functions as they currently exist. Making this change outright would break those applications. Maybe an incremental change would work better. For instance, add an optional kwarg like pack (default to False) which would make the functions return the packed tuples row-wise instead of column-wise as they currently are. This is like the inverse of the unpack option in numpy.genfromtxt. This change would make the new behavior opt-in at the beginning, then in later releases the default value could be changed to True making the old behavior opt-in instead, and finally maybe even completely deprecate the old behavior later on down the line.