Fix slicing bugs

Author: eslavich

asdf-core/src/main/java/org/asdfformat/asdf/Asdf.java

@@ -28,7 +28,7 @@ public class Asdf {
      * @param path path to a .asdf file
      * @return open ASDF file
      */
-    AsdfFile open(final Path path) throws IOException {
+    public static AsdfFile open(final Path path) throws IOException {
         final RandomAccessFile file = new RandomAccessFile(path.toFile(), "r");
         try {
             final LowLevelFormat lowLevelFormat = LowLevelFormats.fromFile(file);

asdf-core/src/main/java/org/asdfformat/asdf/ndarray/Slice.java

@@ -7,7 +7,7 @@
 public interface Slice {
     void validate(int originalLength);
 
-    int computeNewOffset(int originalOffset, int originalStride);
+    int computeNewOffset(int originalLength, int originalOffset, int originalStride);
 
     int computeNewLength(int originalLength);
 

asdf-core/src/main/java/org/asdfformat/asdf/ndarray/Slices.java

@@ -17,24 +17,34 @@ public static Slice all() {
         return AllSlice.instance();
     }
 
+    /**
+     * Select a range of a dimension, beginning at startIndexInclusive
+     * and extending to the end of the array.
+     * @param startIndexInclusive inclusive start index
+     * @return slice
+     */
+    public static Slice range(int startIndexInclusive) {
+        return RangeSlice.of(startIndexInclusive, null);
+    }
+
     /**
      * Select a range of a dimension.
      * @param startIndexInclusive inclusive start index
-     * @param endIndexExclusive exclusive end index
+     * @param endIndexExclusive exclusive end index (or null for the end of the array)
      * @return slice
      */
-    public static Slice range(int startIndexInclusive, int endIndexExclusive) {
+    public static Slice range(int startIndexInclusive, Integer endIndexExclusive) {
         return RangeSlice.of(startIndexInclusive, endIndexExclusive);
     }
 
     /**
      * Select a range of a dimension, with spacing between selected values.
      * @param startIndexInclusive inclusive start index
-     * @param endIndexExclusive exclusive end index
+     * @param endIndexExclusive exclusive end index (or null for the end of the array)
      * @param step spacing between values
      * @return slice
      */
-    public static Slice range(int startIndexInclusive, int endIndexExclusive, int step) {
+    public static Slice range(int startIndexInclusive, Integer endIndexExclusive, int step) {
         return RangeSlice.of(startIndexInclusive, endIndexExclusive, step);
     }
 

asdf-core/src/main/java/org/asdfformat/asdf/ndarray/impl/AllSlice.java

@@ -20,7 +20,7 @@ public void validate(final int originalLength) {
     }
 
     @Override
-    public int computeNewOffset(final int originalOffset, final int originalStride) {
+    public int computeNewOffset(final int originalLength, final int originalOffset, final int originalStride) {
         return originalOffset;
     }
 

asdf-core/src/main/java/org/asdfformat/asdf/ndarray/impl/AtSlice.java

@@ -9,18 +9,16 @@
 @AllArgsConstructor(access = AccessLevel.PRIVATE)
 public class AtSlice implements Slice {
     public static AtSlice of(final int index) {
-        if (index < 0) {
-            throw new IllegalArgumentException("Cannot slice at index < 0");
-        }
-
         return new AtSlice(index);
     }
 
     private final int index;
 
     @Override
     public void validate(final int originalLength) {
-        if (index >= originalLength) {
+        final int resolvedIndex = resolveIndex(originalLength);
+
+        if (resolvedIndex < 0 || resolvedIndex >= originalLength) {
             throw new IllegalArgumentException(
                     String.format(
                             "%s out of range for dimension of length %d",
@@ -32,8 +30,9 @@ public void validate(final int originalLength) {
     }
 
     @Override
-    public int computeNewOffset(final int originalOffset, final int originalStride) {
-        return originalOffset + index * originalStride;
+    public int computeNewOffset(final int originalLength, final int originalOffset, final int originalStride) {
+        final int resolvedIndex = resolveIndex(originalLength);
+        return originalOffset + resolvedIndex * originalStride;
     }
 
     @Override
@@ -45,4 +44,12 @@ public int computeNewLength(final int originalLength) {
     public int computeNewStride(final int originalStride) {
         return originalStride;
     }
+
+    private int resolveIndex(final int length) {
+        if (index >= 0) {
+            return index;
+        }
+
+        return length + index;
+    }
 }

asdf-core/src/main/java/org/asdfformat/asdf/ndarray/impl/NdArrayBase.java

@@ -81,7 +81,15 @@ public NdArray<T> slice(final Slice... slices) {
             sliceList.add(Slices.all());
         }
 
-        final int newOffset = sliceList.get(sliceList.size() - 1).computeNewOffset(offset, strides[strides.length - 1]);
+        for (int i = 0; i < shape.length; i++) {
+            sliceList.get(i).validate(shape[i]);
+        }
+
+        int newOffset = offset;
+        for (int i = 0; i < shape.length; i++) {
+            newOffset += sliceList.get(i).computeNewOffset(shape[i], 0, strides[i]);
+        }
+
         final List<Integer> newShape = new ArrayList<>(shape.length);
         final List<Integer> newStrides = new ArrayList<>(shape.length);
 

asdf-core/src/main/java/org/asdfformat/asdf/ndarray/impl/RangeSlice.java

@@ -9,40 +9,27 @@
 @AllArgsConstructor(access = AccessLevel.PRIVATE)
 public class RangeSlice implements Slice {
     private final int startIndexInclusive;
-    private final int endIndexExclusive;
+    private final Integer endIndexExclusive;
     private final int step;
 
-    public static RangeSlice of(final int startIndexInclusive, final int endIndexExclusive, final int step) {
-        if (startIndexInclusive < 0) {
-            throw new IllegalArgumentException("Cannot slice at start index < 0");
-        }
-
-        if (endIndexExclusive < 0) {
-            throw new IllegalArgumentException("Cannot slice at end index < 0");
-        }
-
+    public static RangeSlice of(final int startIndexInclusive, final Integer endIndexExclusive, final int step) {
         if (step < 1) {
             throw new IllegalArgumentException("Cannot slice with step < 1");
         }
 
-        if (startIndexInclusive > endIndexExclusive) {
-            throw new IllegalArgumentException("Cannot slice with start index > end index");
-        }
-
-        if (endIndexExclusive == startIndexInclusive) {
-            throw new IllegalArgumentException("Empty slice");
-        }
-
         return new RangeSlice(startIndexInclusive, endIndexExclusive, step);
     }
 
-    public static RangeSlice of(final int startIndexInclusive, final int endIndexExclusive) {
+    public static RangeSlice of(final int startIndexInclusive, final Integer endIndexExclusive) {
         return of(startIndexInclusive, endIndexExclusive, 1);
     }
 
     @Override
     public void validate(final int originalLength) {
-        if (startIndexInclusive >= originalLength || endIndexExclusive > originalLength) {
+        final int resolvedStartIndexInclusive = resolveIndex(originalLength, startIndexInclusive);
+        final int resolvedEndIndexInclusive = resolveIndex(originalLength, endIndexExclusive);
+
+        if (resolvedStartIndexInclusive < 0 || resolvedStartIndexInclusive >= originalLength || resolvedEndIndexInclusive < 0 || resolvedEndIndexInclusive > originalLength) {
             throw new IllegalArgumentException(
                     String.format(
                             "%s out of range for dimension of length %d",
@@ -51,20 +38,42 @@ public void validate(final int originalLength) {
                     )
             );
         }
+
+        if (resolvedEndIndexInclusive <= resolvedStartIndexInclusive) {
+            throw new IllegalArgumentException(
+                    String.format(
+                            "%s is empty interval for dimension of length %d",
+                            this,
+                            originalLength
+                    )
+            );
+        }
     }
 
     @Override
-    public int computeNewOffset(final int originalOffset, final int originalStride) {
-        return originalOffset + startIndexInclusive * originalStride;
+    public int computeNewOffset(final int originalLength, final int originalOffset, final int originalStride) {
+        return originalOffset + resolveIndex(originalLength, startIndexInclusive) * originalStride;
     }
 
     @Override
     public int computeNewLength(final int originalLength) {
-        return (endIndexExclusive - startIndexInclusive) / step;
+        return (resolveIndex(originalLength, endIndexExclusive) - resolveIndex(originalLength, startIndexInclusive)) / step;
     }
 
     @Override
     public int computeNewStride(final int originalStride) {
         return originalStride * step;
     }
+
+    private int resolveIndex(final int length, final Integer index) {
+        if (index == null) {
+            return length;
+        }
+
+        if (index >= 0) {
+            return index;
+        }
+
+        return length + index;
+    }
 }

pom.xml

@@ -11,7 +11,7 @@
   <properties>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
     <maven.compiler.release>8</maven.compiler.release>
-    <revision>1.0.0-SNAPSHOT</revision>
+    <revision>0.1-alpha-1</revision>
     <lombok.version>1.18.36</lombok.version>
     <snakeyaml.version>2.4</snakeyaml.version>
     <maven-clean-plugin.version>3.4.0</maven-clean-plugin.version>