Thanks for setting a dedicated PR, @lorentzenchr.

Here are some remarks.

This is an.. interesting hack. I experimented with writing using:

def _write_one(NUM_TYPES[:] x):
    x[0] = 100

The following test segfaults:

def test_readonly_array_wrapper_write():
    x = np.arange(10).astype(np.int32)
    x_readonly = create_memmap_backed_data(x)

    x_readonly_warp = ReadonlyArrayWrapper(x_readonly)
    _write_one(x_readonly_warp)

While using a flag works and writes to the underlying array:

def test_readonly_array_wrapper_write_flag():

    x = np.arange(10).astype(np.int32)
    x_readonly = x.copy()
    x_readonly.flags["WRITEABLE"] = False

    x_readonly_warp = ReadonlyArrayWrapper(x_readonly)
    _write_one(x_readonly_warp)
    assert x_readonly[0] == 100

So if we want to use this hack, we need to make sure to test with memmaped data.

I played with it a bit. Looks like an acceptable temporary workaround for me. I'm however a bit uncomfortable to add a new unused utility class to the codebase. There's a risk that we never follow up with PRs using it and it stays forever as unused code. I'd rather have the PR include at least one use of the new class.

I tried to use it in KMeans and it does not make the code more complex, just wrapping X at 2 places. I realized that KMeans (and MiniBatchKMeans) almost already handled the readonly case because of necessary copies before accessing cython code (the only issue was for predict / score). I ran some benchmarks and did not notice any speed change.

I directly pushed the commit to use it in KMeans here but feel free to revert if you think it hurts the PR.

It is tested for both (see tests):

The test checks for a Cython function that does not write to the buffer, which is good to have.

I was thinking of ways to prevents us from accidentally defining Cython functions that writes to ReadonlyWrapper. Passing in memmapped data is the easy choice since it segfaults. In practice when reviewing PRs, we need to make sure the Cython function with ReadonlyWrapper is tested with memmapped data.

Summary of thoughts:

1. This workaround makes for a nicer developer experience since memoryviews can be passed all the way down. This enables nicer indexing and we no longer need to do stride math.
2. We have to be sure that the Cython function using plain memoryviews do not write to the buffer. In the case of KMeans, we need to depend on the common tests with memmapped data. This adds some maintenance overhead.
3. The caller needs to know that the Cython function does not write to the buffer in order to safely create ReadonlyArrayWrapper.

Overall, I am slightly in favor (+0.25). (I like seeing memoryviews everywhere)

The KMeans use case is convincing, the source code is more natural to follow.

The common tests should already cover the majority of the test cases that @thomasjpfan want.

+1 for merge on my side.

I am convinced.