Repository: scipy/scipy
Pull request: BUG Enables find_peaks to work on read-only data
<!-- 
Thanks for contributing a pull request! Please ensure that
your PR satisfies the checklist before submitting:
http://scipy.github.io/devdocs/dev/contributor/development_workflow.html#checklist-before-submitting-a-pr

Also, please name and describe your PR as you would write a
commit message:
http://scipy.github.io/devdocs/dev/contributor/development_workflow.html#writing-the-commit-message

Note that we are a team of volunteers; we appreciate your
patience during the review process.

Again, thanks for contributing!
-->

#### Reference issue
<!--Example: Closes gh-WXYZ.-->
Closes gh-16197

#### What does this implement/fix?
<!--Please explain your changes.-->
This PR enables the Cython to work on readonly memoryviews.


Problem statement:
BUG: find_peaks() fails if x.flags.writable = False
### Describe your issue.

If the writable flag has been set to False on a np.array, then find_peaks() fails with a ValueError: buffer source array is read-only.
 
It seems that the View.MemoryView functions require writable arrays, but then why are they called "View"?


### Reproducing Code Example

```python
import numpy, scipy.signal

x = numpy.linspace(0,1,100)
x.flags.writeable = False
p = scipy.signal.find_peaks(x)
```


### Error message

```shell
Traceback (most recent call last):
  File "<ipython-input-18-b0e8a4dbfe44>", line 1, in <module>
    p=scipy.signal.find_peaks(x)
  File "/usr/lib/python3/dist-packages/scipy/signal/_peak_finding.py", line 936, in find_peaks
    peaks, left_edges, right_edges = _local_maxima_1d(x)
  File "_peak_finding_utils.pyx", line 20, in scipy.signal._peak_finding_utils._local_maxima_1d
  File "stringsource", line 658, in View.MemoryView.memoryview_cwrapper
  File "stringsource", line 349, in View.MemoryView.memoryview.__cinit__
ValueError: buffer source array is read-only
```


### SciPy/NumPy/Python version information

1.8.0 1.21.5 sys.version_info(major=3, minor=10, micro=4, releaselevel='final', serial=0)

Hints from issue discussion:
Indeed, we should use `const` to indicate that read-only arrays are permissible since the function doesn't actually write back into the input array. [This feature](https://cython.readthedocs.io/en/latest/src/userguide/memoryviews.html#read-only-views) isn't exactly _new_ to Cython, but I expect that it didn't exist at the time `_local_maxima_1d()` was first written.

Commit message: CLN Better variable name

Patch to review:
diff --git a/scipy/signal/_peak_finding_utils.pyx b/scipy/signal/_peak_finding_utils.pyx
index 4a523f135ca2..60cef5736bf3 100644
--- a/scipy/signal/_peak_finding_utils.pyx
+++ b/scipy/signal/_peak_finding_utils.pyx
@@ -17,7 +17,7 @@ __all__ = ['_local_maxima_1d', '_select_by_peak_distance', '_peak_prominences',
            '_peak_widths']
 
 
-def _local_maxima_1d(np.float64_t[::1] x not None):
+def _local_maxima_1d(const np.float64_t[::1] x not None):
     """
     Find local maxima in a 1D array.
 
@@ -163,7 +163,7 @@ class PeakPropertyWarning(RuntimeWarning):
     pass
 
 
-def _peak_prominences(np.float64_t[::1] x not None,
+def _peak_prominences(const np.float64_t[::1] x not None,
                       np.intp_t[::1] peaks not None,
                       np.intp_t wlen):
     """
@@ -261,7 +261,7 @@ def _peak_prominences(np.float64_t[::1] x not None,
     return prominences.base, left_bases.base, right_bases.base
 
 
-def _peak_widths(np.float64_t[::1] x not None,
+def _peak_widths(const np.float64_t[::1] x not None,
                  np.intp_t[::1] peaks not None,
                  np.float64_t rel_height,
                  np.float64_t[::1] prominences not None,
diff --git a/scipy/signal/tests/test_peak_finding.py b/scipy/signal/tests/test_peak_finding.py
index 57ba16181de5..1dee4e87fad5 100644
--- a/scipy/signal/tests/test_peak_finding.py
+++ b/scipy/signal/tests/test_peak_finding.py
@@ -135,7 +135,7 @@ def test_exceptions(self):
         """Test input validation and raised exceptions."""
         with raises(ValueError, match="wrong number of dimensions"):
             _local_maxima_1d(np.ones((1, 1)))
-        with raises(ValueError, match="expected 'float64_t'"):
+        with raises(ValueError, match="expected 'const float64_t'"):
             _local_maxima_1d(np.ones(1, dtype=int))
         with raises(TypeError, match="list"):
             _local_maxima_1d([1., 2.])
@@ -770,6 +770,26 @@ def test_wlen_smaller_plateau(self):
         for key in ("left_bases", "right_bases", "left_ips", "right_ips"):
             assert_equal(props[key], peaks)
 
+    @pytest.mark.parametrize("kwargs", [
+        {},
+        {"distance": 3.0},
+        {"prominence": (None, None)},
+        {"width": (None, 2)},
+
+    ])
+    def test_readonly_array(self, kwargs):
+        """
+        Test readonly arrays are accepted.
+        """
+        x = np.linspace(0, 10, 15)
+        peaks, _ = find_peaks(x)
+
+        x_readonly = x.copy()
+        x_readonly.flags.writeable = False
+        peaks_readonly, _ = find_peaks(x_readonly, **kwargs)
+
+        assert_allclose(peaks, peaks_readonly)
+
 
 class TestFindPeaksCwt:
 


Code review comments:
diff_hunk: @@ -770,6 +770,26 @@ def test_wlen_smaller_plateau(self):
         for key in ("left_bases", "right_bases", "left_ips", "right_ips"):
             assert_equal(props[key], peaks)
 
+    @pytest.mark.parametrize("kwargs", [
+        {},
+        {"distance": 3.0},
+        {"prominence": (None, None)},
+        {"width": (None, 2)},
+
+    ])
+    def test_readonly_array(self, kwargs):
+        """
+        Test readonly arrays are accepted.
+        """
+        x = np.linspace(0, 10, 15)
+        peaks, _ = find_peaks(x)
+
+        x_readonly = x.copy()
path: scipy/signal/tests/test_peak_finding.py
text: @user1:
`x` could change, let's move it up 😃