Repository: jtablesaw/tablesaw
Pull request #422 (2018-12-01 23:47:40)

Add sorting on negative indices + some refactoring
Thanks for contributing.

- [x] Tick to sign-off your agreement to the [Developer Certificate of Origin (DCO) 1.1](https://developercertificate.org)

## Description

Possibility of sorting on negative indices (the was a TODO comment) and tests to that method.
I have also done some refactoring.

## Testing

Yes

lina2002: @lwhite1 would be great if you could take a look!
Maybe you have suggestion as well what would be most beneficial to do.
benmccann: @lina2002 can you try rebasing against master and see if it fixes the Codacy issues?
lina2002: @benmccann sure and looks like it does

Patch:
diff --git a/core/src/main/java/tech/tablesaw/api/BooleanColumn.java b/core/src/main/java/tech/tablesaw/api/BooleanColumn.java
index 1a2f267b..687ca3f1 100644
--- a/core/src/main/java/tech/tablesaw/api/BooleanColumn.java
+++ b/core/src/main/java/tech/tablesaw/api/BooleanColumn.java
@@ -802,9 +802,9 @@ public class BooleanColumn extends AbstractColumn<Boolean> implements BooleanMap
 
     @Override
     public BooleanColumn fillWith(BooleanIterable iterable) {
-        BooleanIterator iterator = null;
+        BooleanIterator iterator = iterable.iterator();
         for (int r = 0; r < size(); r++) {
-            if (iterator == null || (!iterator.hasNext())) {
+            if (!iterator.hasNext()) {
                 iterator = iterable.iterator();
                 if (!iterator.hasNext()) {
                     break;
diff --git a/core/src/main/java/tech/tablesaw/api/DateColumn.java b/core/src/main/java/tech/tablesaw/api/DateColumn.java
index 788ce31b..8b3706e7 100644
--- a/core/src/main/java/tech/tablesaw/api/DateColumn.java
+++ b/core/src/main/java/tech/tablesaw/api/DateColumn.java
@@ -590,9 +590,9 @@ public class DateColumn extends AbstractColumn<LocalDate> implements DateFilters
     }
 
     private DateColumn fillWith(int count, Iterable<LocalDate> iterable, Consumer<LocalDate> acceptor) {
-        Iterator<LocalDate> iterator = null;
+        Iterator<LocalDate> iterator = iterable.iterator();
         for (int r = 0; r < count; r++) {
-            if (iterator == null || (!iterator.hasNext())) {
+            if (!iterator.hasNext()) {
                 iterator = iterable.iterator();
                 if (!iterator.hasNext()) {
                     break;
diff --git a/core/src/main/java/tech/tablesaw/api/DateTimeColumn.java b/core/src/main/java/tech/tablesaw/api/DateTimeColumn.java
index df274dd1..249384a9 100644
--- a/core/src/main/java/tech/tablesaw/api/DateTimeColumn.java
+++ b/core/src/main/java/tech/tablesaw/api/DateTimeColumn.java
@@ -627,9 +627,9 @@ public class DateTimeColumn extends AbstractColumn<LocalDateTime>
     }
 
     private DateTimeColumn fillWith(int count, Iterable<LocalDateTime> iterable, Consumer<LocalDateTime> acceptor) {
-        Iterator<LocalDateTime> iterator = null;
+        Iterator<LocalDateTime> iterator = iterable.iterator();
         for (int r = 0; r < count; r++) {
-            if (iterator == null || (!iterator.hasNext())) {
+            if (!iterator.hasNext()) {
                 iterator = iterable.iterator();
                 if (!iterator.hasNext()) {
                     break;
diff --git a/core/src/main/java/tech/tablesaw/api/DoubleColumn.java b/core/src/main/java/tech/tablesaw/api/DoubleColumn.java
index 08a655d1..7a4c66a0 100644
--- a/core/src/main/java/tech/tablesaw/api/DoubleColumn.java
+++ b/core/src/main/java/tech/tablesaw/api/DoubleColumn.java
@@ -451,7 +451,7 @@ public class DoubleColumn extends NumberColumn<Double> implements NumberFillers<
     public DoubleColumn fillWith(final DoubleRangeIterable iterable) {
 	DoubleIterator iterator = iterable.iterator();
         for (int r = 0; r < size(); r++) {
-            if (iterator == null || (!iterator.hasNext())) {
+            if (!iterator.hasNext()) {
                 iterator = iterable.iterator();
                 if (!iterator.hasNext()) {
                     break;
diff --git a/core/src/main/java/tech/tablesaw/api/NumberColumn.java b/core/src/main/java/tech/tablesaw/api/NumberColumn.java
index 57c78d0e..e750b04b 100644
--- a/core/src/main/java/tech/tablesaw/api/NumberColumn.java
+++ b/core/src/main/java/tech/tablesaw/api/NumberColumn.java
@@ -15,14 +15,10 @@ public abstract class NumberColumn<T extends Number> extends AbstractColumn<T> i
 
     protected Locale locale;
 
-    protected final IntComparator comparator = new IntComparator() {
-
-        @Override
-        public int compare(final int r1, final int r2) {
-            final double f1 = getDouble(r1);
-            final double f2 = getDouble(r2);
-            return Double.compare(f1, f2);
-        }
+    protected final IntComparator comparator = (r1, r2) -> {
+        final double f1 = getDouble(r1);
+        final double f2 = getDouble(r2);
+        return Double.compare(f1, f2);
     };
 
     protected NumberColumn(final ColumnType type, final String name) {
diff --git a/core/src/main/java/tech/tablesaw/api/Table.java b/core/src/main/java/tech/tablesaw/api/Table.java
index 33dc55d5..f248ca42 100644
--- a/core/src/main/java/tech/tablesaw/api/Table.java
+++ b/core/src/main/java/tech/tablesaw/api/Table.java
@@ -44,8 +44,8 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
-import java.util.stream.Collectors;
 
+import static java.util.stream.Collectors.toList;
 import static tech.tablesaw.aggregate.AggregateFunctions.countMissing;
 import static tech.tablesaw.selection.Selection.selectNRowsAtRandom;
 
@@ -313,9 +313,7 @@ public class Table extends Relation implements Iterable<Row> {
      * Returns a List of the names of all the columns in this table
      */
     public List<String> columnNames() {
-        List<String> names = new ArrayList<>(columnList.size());
-        names.addAll(columnList.stream().map(Column::name).collect(Collectors.toList()));
-        return names;
+        return columnList.stream().map(Column::name).collect(toList());
     }
 
     /**
@@ -435,14 +433,18 @@ public class Table extends Relation implements Iterable<Row> {
     }
 
     /**
-     * Sorts this table into a new table on the columns indexed in ascending order
+     * Sorts this table into a new table on the columns indexed
      * <p>
-     * TODO(lwhite): Rework this so passing an negative number does a descending sort
+     * if index is negative then sort that column in decending order otherwise sort ascending
      */
     public Table sortOn(int... columnIndexes) {
         List<String> names = new ArrayList<>();
         for (int i : columnIndexes) {
-            names.add(columnList.get(i).name());
+            if (i >= 0) {
+                names.add(columnList.get(i).name());
+            } else {
+                names.add("-" + columnList.get(-i).name());
+            }
         }
         return sortOn(names.toArray(new String[names.size()]));
     }
@@ -455,34 +457,19 @@ public class Table extends Relation implements Iterable<Row> {
     public Table sortOn(String... columnNames) {
 
         Sort key = null;
-        List<String> names = new ArrayList<>();
-        for (String name : columnNames()) {
-            names.add(name.toUpperCase());
-        }
+        List<String> names = columnNames().stream().map(String::toUpperCase).collect(toList());
 
         for (String columnName : columnNames) {
-            Sort.Order order;
-            if (names.contains(columnName.toUpperCase())) {
-                // the column name has not been annotated with a prefix.
-                order = Sort.Order.ASCEND;
-            } else {
-
+            Sort.Order order = Sort.Order.ASCEND;
+            if (!names.contains(columnName.toUpperCase())) {
+                // the column name has been annotated with a prefix.
                 // get the prefix which could be - or +
                 String prefix = columnName.substring(0, 1);
 
                 // remove - prefix so provided name matches actual column name
                 columnName = columnName.substring(1, columnName.length());
 
-                switch (prefix) {
-                    case "+":
-                        order = Sort.Order.ASCEND;
-                        break;
-                    case "-":
-                        order = Sort.Order.DESCEND;
-                        break;
-                    default:
-                        throw new IllegalStateException("Column prefix: " + prefix + " is unknown.");
-                }
+                order = getOrder(prefix);
             }
 
             if (key == null) { // key will be null the first time through
@@ -494,6 +481,21 @@ public class Table extends Relation implements Iterable<Row> {
         return sortOn(key);
     }
 
+    private Sort.Order getOrder(String prefix) {
+        Sort.Order order;
+        switch (prefix) {
+            case "+":
+                order = Sort.Order.ASCEND;
+                break;
+            case "-":
+                order = Sort.Order.DESCEND;
+                break;
+            default:
+                throw new IllegalStateException("Column prefix: " + prefix + " is unknown.");
+        }
+        return order;
+    }
+
     /**
      * Returns a copy of this table sorted in the order of the given column names, in ascending order
      */
@@ -541,13 +543,10 @@ public class Table extends Relation implements Iterable<Row> {
     public Table sortOn(Comparator<Row> rowComparator) {
         Row row1 = new Row(this);
         Row row2 = new Row(this);
-        return sortOn(new IntComparator() {
-            @Override
-            public int compare(int k1, int k2) {
-                row1.at(k1);
-                row2.at(k2);
-                return rowComparator.compare(row1, row2);
-            }
+        return sortOn((IntComparator) (k1, k2) -> {
+            row1.at(k1);
+            row2.at(k2);
+            return rowComparator.compare(row1, row2);
         });
     }
 
@@ -956,19 +955,20 @@ public class Table extends Relation implements Iterable<Row> {
      * Applies the operation in {@code doable} to every row in the table
      */
     public void stepWithRows(Consumer<Row[]> rowConsumer, int n) {
-        if (!isEmpty()) {
-            Row[] rows = new Row[n];
-            for (int i = 0; i < n; i++) {
-                rows[i] = new Row(this);
-            }
+        if (isEmpty()) {
+            return;
+        }
+        Row[] rows = new Row[n];
+        for (int i = 0; i < n; i++) {
+            rows[i] = new Row(this);
+        }
 
-            int max = rowCount() - n;
-            for (int i = 0; i <= max; i++) {
-                for (int r = 0; r < n; r++) {
-                    rows[r].at(i + r);
-                }
-                rowConsumer.accept(rows);
+        int max = rowCount() - n;
+        for (int i = 0; i <= max; i++) {
+            for (int r = 0; r < n; r++) {
+                rows[r].at(i + r);
             }
+            rowConsumer.accept(rows);
         }
     }
 
@@ -976,15 +976,16 @@ public class Table extends Relation implements Iterable<Row> {
      * Applies the function in {@code pairs} to each consecutive pairs of rows in the table
      */
     public void doWithRows(Pairs pairs) {
+        if (isEmpty()) {
+            return;
+        }
         Row row1 = new Row(this);
         Row row2 = new Row(this);
-        if (!isEmpty()) {
-            int max = rowCount();
-            for (int i = 1; i < max; i++) {
-                row1.at(i - 1);
-                row2.at(i);
-                pairs.doWithPair(row1, row2);
-            }
+        int max = rowCount();
+        for (int i = 1; i < max; i++) {
+            row1.at(i - 1);
+            row2.at(i);
+            pairs.doWithPair(row1, row2);
         }
     }
 
@@ -992,16 +993,17 @@ public class Table extends Relation implements Iterable<Row> {
      * Applies the function in {@code pairs} to each consecutive pairs of rows in the table
      */
     public void doWithRowPairs(Consumer<RowPair> pairConsumer) {
+        if (isEmpty()) {
+            return;
+        }
         Row row1 = new Row(this);
         Row row2 = new Row(this);
         RowPair pair = new RowPair(row1, row2);
-        if (!isEmpty()) {
-            int max = rowCount();
-            for (int i = 1; i < max; i++) {
-                row1.at(i - 1);
-                row2.at(i);
-                pairConsumer.accept(pair);
-            }
+        int max = rowCount();
+        for (int i = 1; i < max; i++) {
+            row1.at(i - 1);
+            row2.at(i);
+            pairConsumer.accept(pair);
         }
     }
 
@@ -1010,19 +1012,20 @@ public class Table extends Relation implements Iterable<Row> {
      * This can be used, for example, to calculate a running average of in rows
      */
     public void rollWithRows(Consumer<Row[]> rowConsumer, int n) {
-        if (!isEmpty()) {
-            Row[] rows = new Row[n];
-            for (int i = 0; i < n; i++) {
-                rows[i] = new Row(this);
-            }
+        if (isEmpty()) {
+            return;
+        }
+        Row[] rows = new Row[n];
+        for (int i = 0; i < n; i++) {
+            rows[i] = new Row(this);
+        }
 
-            int max = rowCount() - (n - 2);
-            for (int i = 1; i < max; i++) {
-                for (int r = 0; r < n; r++) {
-                    rows[r].at(i + r - 1);
-                }
-                rowConsumer.accept(rows);
+        int max = rowCount() - (n - 2);
+        for (int i = 1; i < max; i++) {
+            for (int r = 0; r < n; r++) {
+                rows[r].at(i + r - 1);
             }
+            rowConsumer.accept(rows);
         }
     }
 
diff --git a/core/src/main/java/tech/tablesaw/api/TimeColumn.java b/core/src/main/java/tech/tablesaw/api/TimeColumn.java
index 60189314..3b26133f 100644
--- a/core/src/main/java/tech/tablesaw/api/TimeColumn.java
+++ b/core/src/main/java/tech/tablesaw/api/TimeColumn.java
@@ -605,9 +605,9 @@ public class TimeColumn extends AbstractColumn<LocalTime>
     }
 
     private TimeColumn fillWith(int count, Iterable<LocalTime> iterable, Consumer<LocalTime> acceptor) {
-        Iterator<LocalTime> iterator = null;
+        Iterator<LocalTime> iterator = iterable.iterator();
         for (int r = 0; r < count; r++) {
-            if (iterator == null || (!iterator.hasNext())) {
+            if (!iterator.hasNext()) {
                 iterator = iterable.iterator();
                 if (!iterator.hasNext()) {
                     break;
diff --git a/core/src/main/java/tech/tablesaw/table/Relation.java b/core/src/main/java/tech/tablesaw/table/Relation.java
index 26aa2294..563dd3aa 100644
--- a/core/src/main/java/tech/tablesaw/table/Relation.java
+++ b/core/src/main/java/tech/tablesaw/table/Relation.java
@@ -39,6 +39,7 @@ import tech.tablesaw.sorting.comparators.DescendingIntComparator;
 
 import java.io.ByteArrayOutputStream;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -332,11 +333,7 @@ public abstract class Relation {
      * Returns all the NumericColumns in the relation
      */
     public List<NumericColumn<?>> numericColumns() {
-        List<NumericColumn<?>> cols = new ArrayList<>();
-        for (NumericColumn<?> c : numberColumns()) {
-            cols.add(c);
-        }
-        return cols;
+        return Arrays.asList(numberColumns());
     }
 
     /**
diff --git a/jsplot/src/examples/java/tech/tablesaw/plotly/MultiPlotExample.java b/jsplot/src/examples/java/tech/tablesaw/plotly/MultiPlotExample.java
new file mode 100644
index 00000000..30ab6657
--- /dev/null
+++ b/jsplot/src/examples/java/tech/tablesaw/plotly/MultiPlotExample.java
@@ -0,0 +1,114 @@
+package tech.tablesaw.plotly;
+
+import tech.tablesaw.api.NumberColumn;
+import tech.tablesaw.api.Table;
+import tech.tablesaw.plotly.components.Axis;
+import tech.tablesaw.plotly.components.Figure;
+import tech.tablesaw.plotly.components.Layout;
+import tech.tablesaw.plotly.display.Browser;
+import tech.tablesaw.plotly.traces.ScatterTrace;
+import tech.tablesaw.plotly.traces.Trace;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.List;
+
+public class MultiPlotExample {
+
+    public static void main(String[] args) throws Exception {
+
+        /*
+         Preliminaries:
+         1. Get a table
+         2. Split the data into two tables, one for each league
+         3. Get the columns we're going to use in the plot as x1, y1, x2, y2
+         4. Create a layout for each plot
+         5. Create the traces for each plot
+         6. Build a Figure for each plot from the layout and trace
+         7. Create an HTML page as a string
+         8. Write the string to a file
+         9. Open the default desktop Web browser on the file so you can see it
+         */
+
+
+        // 1. Get a table
+        Table baseball = Table.read().csv("../data/baseball.csv");
+
+        // 2. Split the data into two tables, one for each league
+        List<Table> leagueTables = baseball.splitOn("league").asTableList();
+        Table league1 = leagueTables.get(0);
+        Table league2 = leagueTables.get(1);
+
+        // 3. Get the columns we're going to use in the plot as x1, y1, x2, y2
+        NumberColumn<?> x1 = league1.nCol("BA");
+        NumberColumn<?> y1 = league1.nCol("W");
+
+        NumberColumn<?> x2 = league2.nCol("BA");
+        NumberColumn<?> y2 = league2.nCol("W");
+
+        // 4. Create a layout for each plot
+        Layout layout1 = Layout.builder().title("American League Wins vs BA")
+                .xAxis(Axis.builder().title("Batting Average").build())
+                .yAxis(Axis.builder().title("Wins").build())
+                .build();
+
+        Layout layout2 = Layout.builder().title("National League Wins vs BA")
+                .xAxis(Axis.builder().title("Batting Average").build())
+                .yAxis(Axis.builder().title("Wins").build())
+                .build();
+
+        // 5. Create the traces for each plot
+        Trace trace1 = ScatterTrace.builder(x1, y1).build();
+        Trace trace2 = ScatterTrace.builder(x2, y2).build();
+
+        // 6. Build a Figure for each plot from the layout and trace
+        Figure figure1 = new Figure(layout1, trace1);
+        Figure figure2 = new Figure(layout2, trace2);
+
+        // 7. Create an HTML page as a string
+        String divName1 = "plot1";
+        String divName2 = "plot2";
+        String page = makePage(figure1, figure2, divName1, divName2);
+
+        // 8. Write the string to a file
+        File outputFile = Paths.get("multiplot.html").toFile();
+        try {
+            try (FileWriter fileWriter = new FileWriter(outputFile)) {
+                fileWriter.write(page);
+            }
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+
+        // 9. Open the default desktop Web browser on the file so you can see it
+        new Browser().browse(outputFile);
+    }
+
+    private static String makePage(Figure figure1, Figure figure2, String divName1, String divName2) {
+
+        return new StringBuilder()
+                .append(pageTop)
+                .append('\n')
+                .append(figure1.asJavascript(divName1))
+                .append('\n')
+                .append(figure2.asJavascript(divName2))
+                .append('\n')
+                .append(pageBottom)
+                .toString();
+    }
+
+    private static final String pageTop = "<html>\n" +
+            "<head>\n" +
+            "    <title>Multi-plot test</title>\n" +
+            "    <script src=\"https://cdn.plot.ly/plotly-latest.min.js\"></script>\n" +
+            "</head>\n" +
+            "<body>\n" +
+            "<div id='plot1'>\n" +
+            "<div id='plot2'>\n";
+
+    private static final String pageBottom =
+            "</body>\n" +
+            "</html>";
+}


Test patch:
diff --git a/core/src/test/java/tech/tablesaw/SortTest.java b/core/src/test/java/tech/tablesaw/SortTest.java
index adc32c95..1dbaf080 100644
--- a/core/src/test/java/tech/tablesaw/SortTest.java
+++ b/core/src/test/java/tech/tablesaw/SortTest.java
@@ -100,6 +100,20 @@ public class SortTest {
         assertTablesEquals(expectedResults, sortedTable);
     }
 
+    @Test
+    public void testSortOnIndices() {
+        Table sortedTable = unsortedTable.sortOn(IQ_INDEX, DOB_INDEX);
+        Table expectedResults = TestData.SIMPLE_SORTED_DATA_BY_DOUBLE_AND_DATE_ASCENDING.getTable();
+        assertTablesEquals(expectedResults, sortedTable);
+    }
+
+    @Test
+    public void testSortOnIndicesAscendingAndDescending() {
+        Table sortedTable = unsortedTable.sortOn(IQ_INDEX, -DOB_INDEX);
+        Table expectedResults = TestData.SIMPLE_SORTED_DATA_BY_DOUBLE_ASCENDING_AND_THEN_DATE_DESCENDING.getTable();
+        assertTablesEquals(expectedResults, sortedTable);
+    }
+
     @Ignore
     public void testAscendingWithPlusSignNegative() {
         Table sortedTable = unsortedTable.sortOn("+" + columnNames[IQ_INDEX], "-" + columnNames[DOB_INDEX]);
diff --git a/core/src/test/java/tech/tablesaw/api/FloatColumnTest.java b/core/src/test/java/tech/tablesaw/api/FloatColumnTest.java
index 81a22b93..54452624 100644
--- a/core/src/test/java/tech/tablesaw/api/FloatColumnTest.java
+++ b/core/src/test/java/tech/tablesaw/api/FloatColumnTest.java
@@ -30,7 +30,7 @@ public class FloatColumnTest {
     @Test
     public void testAppendFloatObject() {
         final FloatColumn floatColumn = FloatColumn.create("floats");
-        final Float floatObject = Float.valueOf(2.5f);
+        final Float floatObject = 2.5f;
         floatColumn.append(floatObject);
         Assert.assertEquals(floatObject, floatColumn.get(0));
     }