diff --git a/hplc/quant.py b/hplc/quant.py index 49d0941..cf06fb9 100644 --- a/hplc/quant.py +++ b/hplc/quant.py @@ -130,9 +130,10 @@ def crop( Parameters ---------- - time_window : `list` [start, end], optional - The retention time window of the chromatogram to consider for analysis. - If None, the entire time range of the chromatogram will be considered. + time_window : `list` [start, end] + The retention time window of the chromatogram to consider for + analysis. This is required; a `ValueError` is raised if it is not + provided. return_df : `bool` If `True`, the cropped DataFrame is @@ -147,6 +148,9 @@ def crop( You are trying to crop a chromatogram after it has been fit. Make sure that you do this before calling `fit_peaks()` or provide the argument `time_window` to the `fit_peaks()`.""" ) + if time_window is None: + raise ValueError( + "`time_window` must be provided as a list of [start, end].") if len(time_window) != 2: raise ValueError( f"`time_window` must be of length 2 (corresponding to start and end points). Provided list is of length {len(time_window)}." @@ -324,7 +328,7 @@ def _assign_windows( ranges = [] for l, r in zip(_left, _right): _range = np.arange(int(l - buffer), int(r + buffer), 1) - _range = _range[(_range >= 0) & (_range <= len(norm_int))] + _range = _range[(_range >= 0) & (_range < len(norm_int))] ranges.append(_range) # Identiy subset ranges and remove @@ -644,7 +648,6 @@ def deconvolve_peaks( "skew": [-np.inf, np.inf], } # Modify the parameter bounds given arguments - key_inds = {k: i for i, k in enumerate(_param_bounds.keys())} if len(param_bounds) != 0: for p in parorder: if p in param_bounds.keys(): @@ -657,13 +660,16 @@ def deconvolve_peaks( v["location"][i] + p for p in param_bounds[p] ] else: - if (p0[key_inds[p]] >= param_bounds[p][0]) & ( - p0[key_inds[p]] <= param_bounds[p][1] + # `paridx` indexes from the end of `p0`, which + # accumulates 4 entries per peak, so it always + # refers to the peak currently being set up. + if (p0[paridx[p]] >= param_bounds[p][0]) & ( + p0[paridx[p]] <= param_bounds[p][1] ): _param_bounds[p] = param_bounds[p] else: raise ValueError( - f"Bounds for parameter '{p}' [{param_bounds[p]}] is exclusive of initial guess {p0[key_inds[p]]:0.3f} for peak at retention time {p0[1]}." + f"Bounds for parameter '{p}' [{param_bounds[p]}] is exclusive of initial guess {p0[paridx[p]]:0.3f} for peak at retention time {p0[paridx['location']]}." ) # Add peak-specific bounds if provided diff --git a/tests/test_chromatogram.py b/tests/test_chromatogram.py index 8720bfe..d915539 100644 --- a/tests/test_chromatogram.py +++ b/tests/test_chromatogram.py @@ -4,6 +4,7 @@ import numpy as np import matplotlib.pyplot as plt import pytest +import scipy.stats def compare(a, b, tol): @@ -66,14 +67,10 @@ def test_peak_fitting(): chrom_df = pd.read_csv('./tests/test_data/test_fitting_chrom.csv') chrom = hplc.quant.Chromatogram( chrom_df, cols={'time': 'x', 'signal': 'y'}) - try: + with pytest.raises(ValueError): chrom._assign_windows(rel_height=-1) - except ValueError: - assert True - try: + with pytest.raises(ValueError): chrom._assign_windows(rel_height=2) - except ValueError: - assert True chrom_df = pd.read_csv('./tests/test_data/test_fitting_chrom.csv') chrom = hplc.quant.Chromatogram(chrom_df[chrom_df['iter'] == 1], cols={ @@ -204,16 +201,15 @@ def test_crop(): """ data = pd.read_csv('./tests/test_data/test_assessment_chrom.csv') chrom = hplc.quant.Chromatogram(data, cols={'time': 'x', 'signal': 'y'}) - try: + with pytest.raises(ValueError): chrom.crop([1, 2, 3]) - assert False - except ValueError: - assert True - try: + with pytest.raises(RuntimeError): chrom.crop([2, 1]) - assert False - except RuntimeError: - assert True + # A missing/None time window should give a clear ValueError, not a TypeError. + with pytest.raises(ValueError, match='must be provided as a list'): + chrom.crop(None) + with pytest.raises(ValueError, match='must be provided as a list'): + chrom.crop() # Test that a dataframe is returned only if specified. no_returned_df = chrom.crop([10, 20], return_df=False) @@ -246,10 +242,8 @@ def test_deconvolve_peaks(): """ data = pd.read_csv('./tests/test_data/test_assessment_chrom.csv') chrom = hplc.quant.Chromatogram(data, cols={'time': 'x', 'signal': 'y'}) - try: + with pytest.raises(RuntimeError): chrom.deconvolve_peaks() - except RuntimeError: - assert True def test_map_peaks(): @@ -515,3 +509,32 @@ def test_generic_param_bounding(): assert False except ValueError: assert True + + +def test_multipeak_param_bounds_validated_per_peak(): + """ + Regression test for bug B: when a global `param_bounds` is applied to a + window containing more than one peak, the initial-guess-vs-bounds check must + use *each* peak's own guess. Previously it always inspected the first peak's + guess, so a bound that excluded a later peak's guess was silently accepted + (and later surfaced as an opaque scipy error rather than the informative one). + """ + # Two overlapping peaks with clearly different widths that share one window. + t = np.arange(0, 30, 0.01) + sig = (200 * scipy.stats.norm(14.0, 0.2).pdf(t) + + 200 * scipy.stats.norm(15.0, 0.6).pdf(t)) + df = pd.DataFrame({'time': t, 'signal': sig}) + + # Inspect the per-peak scale initial guesses (scale guess = width / 2). + probe = hplc.quant.Chromatogram(df) + probe._assign_windows(buffer=200) + multi = [v for v in probe.window_props.values() if v['num_peaks'] == 2] + assert len(multi) == 1, "expected the two peaks to share a single window" + guesses = sorted(w / 2 for w in multi[0]['width']) + # Bound contains the smaller guess (first peak) but excludes the larger one. + bound = [0, 0.5 * (guesses[0] + guesses[1])] + + chrom = hplc.quant.Chromatogram(df) + with pytest.raises(ValueError, match='exclusive of initial guess'): + chrom.fit_peaks(correct_baseline=False, buffer=200, + param_bounds={'scale': bound}, verbose=False)