Skip to content

Commit

Permalink
Ensure that a jagged slice fits the array's length. (#725)
Browse files Browse the repository at this point in the history
  • Loading branch information
jpivarski committed Feb 10, 2021
1 parent 05945e3 commit 97c7487
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 22 deletions.
14 changes: 12 additions & 2 deletions src/libawkward/array/ByteMaskedArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1560,8 +1560,18 @@ namespace awkward {

template <typename S>
const ContentPtr ByteMaskedArray::getitem_next_jagged_generic(
const Index64& slicestarts, const Index64& slicestops,
const S& slicecontent, const Slice& tail) const {
const Index64& slicestarts,
const Index64& slicestops,
const S& slicecontent,
const Slice& tail) const {
if (slicestarts.length() != length()) {
throw std::invalid_argument(
std::string("cannot fit jagged slice with length ")
+ std::to_string(slicestarts.length()) + std::string(" into ")
+ classname() + std::string(" of size ") + std::to_string(length())
+ FILENAME(__LINE__));
}

int64_t numnull;
std::pair<Index64, Index64> pair = nextcarry_outindex(numnull);
Index64 nextcarry = pair.first;
Expand Down
8 changes: 8 additions & 0 deletions src/libawkward/array/IndexedArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2762,6 +2762,14 @@ namespace awkward {
const Index64& slicestops,
const S& slicecontent,
const Slice& tail) const {
if (slicestarts.length() != length()) {
throw std::invalid_argument(
std::string("cannot fit jagged slice with length ")
+ std::to_string(slicestarts.length()) + std::string(" into ")
+ classname() + std::string(" of size ") + std::to_string(length())
+ FILENAME(__LINE__));
}

if (ISOPTION) {
int64_t numnull;
std::pair<Index64, IndexOf<T>> pair = nextcarry_outindex(numnull);
Expand Down
43 changes: 26 additions & 17 deletions src/libawkward/array/ListArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1781,14 +1781,12 @@ namespace awkward {
const Index64& slicestops,
const SliceArray64& slicecontent,
const Slice& tail) const {
if (starts_.length() < slicestarts.length()) {
util::handle_error(
failure("jagged slice length differs from array length",
kSliceNone,
kSliceNone,
FILENAME_C(__LINE__)),
classname(),
identities_.get());
if (slicestarts.length() != length()) {
throw std::invalid_argument(
std::string("cannot fit jagged slice with length ")
+ std::to_string(slicestarts.length()) + std::string(" into ")
+ classname() + std::string(" of size ") + std::to_string(length())
+ FILENAME(__LINE__));
}
if (stops_.length() < starts_.length()) {
util::handle_error(
Expand Down Expand Up @@ -1843,6 +1841,13 @@ namespace awkward {
const Index64& slicestops,
const SliceMissing64& slicecontent,
const Slice& tail) const {
if (slicestarts.length() != length()) {
throw std::invalid_argument(
std::string("cannot fit jagged slice with length ")
+ std::to_string(slicestarts.length()) + std::string(" into ")
+ classname() + std::string(" of size ") + std::to_string(length())
+ FILENAME(__LINE__));
}
if (starts_.length() < slicestarts.length()) {
util::handle_error(
failure("jagged slice length differs from array length",
Expand Down Expand Up @@ -1924,14 +1929,12 @@ namespace awkward {
const Index64& slicestops,
const SliceJagged64& slicecontent,
const Slice& tail) const {
if (starts_.length() < slicestarts.length()) {
util::handle_error(
failure("jagged slice length differs from array length",
kSliceNone,
kSliceNone,
FILENAME_C(__LINE__)),
classname(),
identities_.get());
if (slicestarts.length() != length()) {
throw std::invalid_argument(
std::string("cannot fit jagged slice with length ")
+ std::to_string(slicestarts.length()) + std::string(" into ")
+ classname() + std::string(" of size ") + std::to_string(length())
+ FILENAME(__LINE__));
}

Index64 outoffsets(slicestarts.length() + 1);
Expand Down Expand Up @@ -1971,7 +1974,13 @@ namespace awkward {
const Index64& slicestops,
const SliceVarNewAxis& slicecontent,
const Slice& tail) const {
throw std::runtime_error("FIXME ListArrayOf<T>::SliceVarNewAxis");
throw std::runtime_error(
"FIXME ListArrayOf<T>::SliceVarNewAxis. "
"2021-02-10 Was this left over from development? If so, it's not getting tested. "
"If anyone out there encounters this error, please report it so that "
"we can properly validate this code path and include it in the tests. "
"https://github.com/scikit-hep/awkward-1.0/issues/new?assignees=&labels=bug+%28unverified%29&template=bug-report.md&title="
);

SliceJagged64 jagged = varaxis_to_jagged(slicecontent);
return getitem_next_jagged(slicestarts, slicestops, jagged, tail);
Expand Down
3 changes: 2 additions & 1 deletion src/libawkward/array/RecordArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1870,7 +1870,8 @@ namespace awkward {
else {
ContentPtrVec contents;
for (auto content : contents_) {
contents.push_back(content.get()->getitem_next_jagged(slicestarts,
ContentPtr trimmed = content.get()->getitem_range_nowrap(0, length());
contents.push_back(trimmed.get()->getitem_next_jagged(slicestarts,
slicestops,
slicecontent,
tail));
Expand Down
6 changes: 4 additions & 2 deletions tests/test_0315-integerindex.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,10 @@ def test_silly_stuff():
a[b]

a = ak.Array([[0, 1, 2], [3, 4], [5, 6], [7]])
b = ak.Array([[0, 2], None])
assert ak.to_list(a[b]) == [[0, 2], None]
### This should not be allowed. I don't know why it was here.
# b = ak.Array([[0, 2], None])
b = ak.Array([[0, 2], None, [1], None])
assert ak.to_list(a[b]) == [[0, 2], None, [6], None]
b = ak.Array([[0, 2], None, None, None, None, None])
with pytest.raises(ValueError):
a[b]
28 changes: 28 additions & 0 deletions tests/test_0723-ensure-that-jagged-slice-fits-array-length.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# BSD 3-Clause License; see https://github.com/scikit-hep/awkward-1.0/blob/main/LICENSE

from __future__ import absolute_import

import pytest # noqa: F401
import numpy as np # noqa: F401
import awkward as ak # noqa: F401


def test():
a = ak.layout.NumpyArray(np.arange(122))
idx = ak.layout.Index64([0, 2, 4, 6, 8, 10, 12])
a = ak.layout.ListOffsetArray64(idx, a)
idx = ak.layout.Index64([0, -1, 1, 2, -1, 3, 4, 5])
a = ak.layout.IndexedOptionArray64(idx, a)
a = ak.Array(a)
with pytest.raises(ValueError):
a[[[0], None]]
assert a[[[0], None, [], [], [], [], [], []]].tolist() == [
[0],
None,
[],
[],
None,
[],
[],
[],
]

0 comments on commit 97c7487

Please sign in to comment.