From 16cb94a3383e8ce5ebb4a13d79347315aac5fa66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 24 Apr 2023 18:35:41 +0200 Subject: [PATCH] python: use flipped() instead of every() with a negative number. That's the way to go forward, StridedBitArrayView doesn't accept signed numbers in every() anymore. Also extracted the slicing code to external helpers in order to avoid duplication. Those will get reused in [Strided]BitArray[View] bindings as well. --- src/python/corrade/containers.cpp | 75 +++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 23 deletions(-) diff --git a/src/python/corrade/containers.cpp b/src/python/corrade/containers.cpp index 22d24bd..6707904 100644 --- a/src/python/corrade/containers.cpp +++ b/src/python/corrade/containers.cpp @@ -41,7 +41,8 @@ namespace { struct Slice { std::size_t start; std::size_t stop; - std::ptrdiff_t step; + bool flip; + std::size_t step; }; Slice calculateSlice(const py::slice& slice, std::size_t containerSize) { @@ -53,14 +54,30 @@ Slice calculateSlice(const py::slice& slice, std::size_t containerSize) { throw py::error_already_set{}; /* If step is negative, start > stop and we have to recalculate */ + /** @todo this doesn't seem to be a guarantee, the assert fires with bad + input as well */ CORRADE_INTERNAL_ASSERT((start <= stop) == (step > 0)); + bool flip = false; if(step < 0) { std::swap(start, stop); start += 1; stop += 1; + step = -step; + flip = true; } - return Slice{std::size_t(start), std::size_t(stop), step}; + return Slice{std::size_t(start), std::size_t(stop), flip, std::size_t(step)}; +} + +template Containers::PyArrayViewHolder arrayViewStridedSlice(const T& self, const Slice& calculated, py::object owner) { + auto sliced = self.slice(calculated.start, calculated.stop); + /* every() currently accepts negative numbers in StridedArrayView, but in + the future it will not, flipped() is the better API. StridedBitArrayView + accepts just an unsigned type. */ + if(calculated.flip) + sliced = sliced.template flipped<0>(); + sliced = sliced.every(calculated.step); + return Containers::pyArrayViewHolder(sliced, calculated.start == calculated.stop ? py::none{} : std::move(owner)); } template bool arrayViewBufferProtocol(T& self, Py_buffer& buffer, int flags) { @@ -152,9 +169,8 @@ template void arrayView(py::class_, Containers /* Non-trivial stride, return a different type */ /** @todo this always assumes bytes for now -- remember the format and provide a checked typed conversion API */ - if(calculated.step != 1) { - auto sliced = Containers::PyStridedArrayView<1, T>{ Containers::stridedArrayView(self)}.slice(calculated.start, calculated.stop).every(calculated.step); - return pyCastButNotShitty(Containers::pyArrayViewHolder(sliced, sliced.size() ? pyObjectHolderFor(self).owner : py::none{})); + if(calculated.step != 1 || calculated.flip) { + return pyCastButNotShitty(arrayViewStridedSlice(Containers::PyStridedArrayView<1, T>{Containers::stridedArrayView(self)}, calculated, pyObjectHolderFor(self).owner)); } /* Usual business */ @@ -385,6 +401,35 @@ template<> struct StridedOperation<4> { } }; +template class Steps, class T> Containers::PyArrayViewHolder stridedArrayViewSlice(const T& self, const typename DimensionsTuple::Type& slice, py::object owner) { + Containers::Size starts; + Containers::Size stops; + Containers::StridedDimensions flips; + Steps steps; + + bool empty = false; + for(std::size_t i = 0; i != dimensions; ++i) { + const Slice calculated = calculateSlice(dimensionsTupleGet(slice, i), self.size()[i]); + starts[i] = calculated.start; + stops[i] = calculated.stop; + steps[i] = calculated.step; + flips[i] = calculated.flip; + steps[i] = calculated.step; + + if(calculated.start == calculated.stop) + empty = true; + } + + auto sliced = self.slice(starts, stops); + /* every() currently accepts negative numbers in StridedArrayView, but in + the future it will not, flipped() is the better API. StridedBitArrayView + accepts just an unsigned type. */ + for(std::size_t i = 0; i != dimensions; ++i) + if(flips[i]) sliced = StridedOperation::flipped(sliced, i); + sliced = sliced.every(steps); + return Containers::pyArrayViewHolder(sliced, empty ? py::none{} : std::move(owner)); +} + template void stridedArrayView(py::class_, Containers::PyArrayViewHolder>>& c) { /* Implicitly convertible from a buffer */ py::implicitly_convertible>(); @@ -459,8 +504,7 @@ template void stridedArrayView(py::class_& self, py::slice slice) { const Slice calculated = calculateSlice(slice, Containers::Size{self.size()}[0]); - const auto sliced = self.slice(calculated.start, calculated.stop).every(calculated.step); - return Containers::pyArrayViewHolder(sliced, calculated.start == calculated.stop ? py::none{} : pyObjectHolderFor(self).owner); + return arrayViewStridedSlice(self, calculated, pyObjectHolderFor(self).owner); }, "Slice the view", py::arg("slice")) /* Fancy operations */ @@ -513,22 +557,7 @@ template void stridedArrayViewND(py::class_& self, const typename DimensionsTuple::Type& slice) { - Containers::Size starts; - Containers::Size stops; - Containers::Stride steps; - - bool empty = false; - for(std::size_t i = 0; i != dimensions; ++i) { - const Slice calculated = calculateSlice(dimensionsTupleGet(slice, i), self.size()[i]); - starts[i] = calculated.start; - stops[i] = calculated.stop; - steps[i] = calculated.step; - - if(calculated.start == calculated.stop) empty = true; - } - - const auto sliced = self.slice(starts, stops).every(steps); - return Containers::pyArrayViewHolder(sliced, empty ? py::none{} : pyObjectHolderFor(self).owner); + return stridedArrayViewSlice(self, slice, pyObjectHolderFor(self).owner); }, "Slice the view", py::arg("slice")) /* Fancy operations */