Repository: jtablesaw/tablesaw
Pull request #719 (2019-11-13 16:46:33)

Pr instant column min max
Thanks for contributing.

- [X] Tick to sign-off your agreement to the [Developer Certificate of Origin (DCO) 1.1](https://developercertificate.org)

## Description
What changed:

1) Fixed a bug in InstantColumn where min would be null if the first value in the column was missing.
2) Update AggregationFunctions to provide a minInstant, maxInstant method for rolling up InstantColumns

## Testing

Did you add a unit test?
Yes: See AggregateFunctionsTest


btrapp: Really loving tablesaw. This is my first github PR ever so if I'm offbase or not following best practices please let me know.  

Patch:
diff --git a/core/src/main/java/tech/tablesaw/aggregate/AggregateFunctions.java b/core/src/main/java/tech/tablesaw/aggregate/AggregateFunctions.java
index bc76420a..d7af16f8 100644
--- a/core/src/main/java/tech/tablesaw/aggregate/AggregateFunctions.java
+++ b/core/src/main/java/tech/tablesaw/aggregate/AggregateFunctions.java
@@ -1,14 +1,17 @@
 package tech.tablesaw.aggregate;
 
+import java.time.Instant;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import org.apache.commons.math3.stat.StatUtils;
 import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics;
 import org.apache.commons.math3.stat.descriptive.moment.Kurtosis;
 import org.apache.commons.math3.stat.descriptive.moment.Skewness;
+
 import tech.tablesaw.api.BooleanColumn;
 import tech.tablesaw.api.DateColumn;
 import tech.tablesaw.api.DateTimeColumn;
+import tech.tablesaw.api.InstantColumn;
 import tech.tablesaw.api.NumericColumn;
 import tech.tablesaw.columns.Column;
 import tech.tablesaw.columns.numbers.DoubleColumnType;
@@ -51,6 +54,21 @@ public class AggregateFunctions {
         }
       };
 
+  public static InstantAggregateFunction maxInstant =
+      new InstantAggregateFunction("Max Instant") {
+        @Override
+        public Instant summarize(InstantColumn column) {
+          return column.max();
+        }
+      };
+  public static InstantAggregateFunction minInstant =
+      new InstantAggregateFunction("Min Instant") {
+        @Override
+        public Instant summarize(InstantColumn column) {
+          return column.min();
+        }
+      };
+
   public static BooleanCountFunction countTrue =
       new BooleanCountFunction("Number True") {
 
diff --git a/core/src/main/java/tech/tablesaw/aggregate/InstantAggregateFunction.java b/core/src/main/java/tech/tablesaw/aggregate/InstantAggregateFunction.java
new file mode 100644
index 00000000..780dbcc2
--- /dev/null
+++ b/core/src/main/java/tech/tablesaw/aggregate/InstantAggregateFunction.java
@@ -0,0 +1,25 @@
+package tech.tablesaw.aggregate;
+
+import java.time.Instant;
+import tech.tablesaw.api.ColumnType;
+import tech.tablesaw.api.InstantColumn;
+
+/** A partial implementation of aggregate functions to summarize over an instant column */
+public abstract class InstantAggregateFunction extends AggregateFunction<InstantColumn, Instant> {
+
+  public InstantAggregateFunction(String name) {
+    super(name);
+  }
+
+  public abstract Instant summarize(InstantColumn column);
+
+  @Override
+  public boolean isCompatibleColumn(ColumnType type) {
+    return type.equals(ColumnType.INSTANT);
+  }
+
+  @Override
+  public ColumnType returnType() {
+    return ColumnType.INSTANT;
+  }
+}
diff --git a/core/src/main/java/tech/tablesaw/api/InstantColumn.java b/core/src/main/java/tech/tablesaw/api/InstantColumn.java
index 7688308a..cdd44746 100644
--- a/core/src/main/java/tech/tablesaw/api/InstantColumn.java
+++ b/core/src/main/java/tech/tablesaw/api/InstantColumn.java
@@ -486,19 +486,18 @@ public class InstantColumn extends AbstractColumn<InstantColumn, Instant>
   }
 
   public Instant max() {
-    long max;
-    if (!isEmpty()) {
-      max = getPackedDateTime(0);
-    } else {
+    if (isEmpty()) {
       return null;
     }
+    long max = Long.MIN_VALUE;
+    boolean allMissing = true;
     for (long aData : data) {
       if (InstantColumnType.missingValueIndicator() != aData) {
-        max = (max > aData) ? max : aData;
+        max = Math.max(max , aData);
+        allMissing = false;
       }
     }
-
-    if (InstantColumnType.missingValueIndicator() == max) {
+    if (allMissing) {
       return null;
     }
     return PackedInstant.asInstant(max);
@@ -512,19 +511,18 @@ public class InstantColumn extends AbstractColumn<InstantColumn, Instant>
 
   @Override
   public Instant min() {
-    long min;
-
-    if (!isEmpty()) {
-      min = getPackedDateTime(0);
-    } else {
+    if (isEmpty()) {
       return null;
     }
+    long min = Long.MAX_VALUE;
+    boolean allMissing = true;
     for (long aData : data) {
       if (InstantColumnType.missingValueIndicator() != aData) {
-        min = (min < aData) ? min : aData;
+        min = Math.min(min, aData);
+        allMissing = false;
       }
     }
-    if (Integer.MIN_VALUE == min) {
+    if (allMissing) {
       return null;
     }
     return PackedInstant.asInstant(min);
diff --git a/jsplot/src/main/java/tech/tablesaw/plotly/components/Layout.java b/jsplot/src/main/java/tech/tablesaw/plotly/components/Layout.java
index 5660ec08..eb598c9f 100644
--- a/jsplot/src/main/java/tech/tablesaw/plotly/components/Layout.java
+++ b/jsplot/src/main/java/tech/tablesaw/plotly/components/Layout.java
@@ -119,6 +119,9 @@ public class Layout {
    */
   private final boolean autoSize;
 
+  private final boolean heightSet;
+  private final boolean widthSet;
+
   /** The width of the plot in pixels */
   private final int width;
 
@@ -178,6 +181,8 @@ public class Layout {
   private Layout(LayoutBuilder builder) {
     this.title = builder.title;
     this.autoSize = builder.autoSize;
+    this.widthSet = builder.widthSet;
+    this.heightSet = builder.heightSet;
     this.decimalSeparator = builder.decimalSeparator;
     this.thousandsSeparator = builder.thousandsSeparator;
     this.dragMode = builder.dragMode;
@@ -222,10 +227,21 @@ public class Layout {
     Map<String, Object> context = new HashMap<>();
     if (!title.equals(DEFAULT_TITLE)) context.put("title", title);
     if (!titleFont.equals(DEFAULT_TITLE_FONT)) context.put("titlefont", titleFont);
-    context.put("width", width);
-    context.put("height", height);
     if (!font.equals(DEFAULT_FONT)) context.put("font", font);
-    if (!autoSize == DEFAULT_AUTO_SIZE) context.put("autosize", autoSize);
+    if (autoSize != DEFAULT_AUTO_SIZE) {
+      context.put("autosize", autoSize);
+      // since autosize is true, we assume the default width / height values are not wanted, not
+      // serialize them, and let Plotly compute them
+      if (widthSet) {
+        context.put("width", width);
+      }
+      if (heightSet) {
+        context.put("height", height);
+      }
+    } else {
+      context.put("width", width);
+      context.put("height", height);
+    }
     if (hoverDistance != DEFAULT_HOVER_DISTANCE) context.put("hoverdistance", hoverDistance);
     if (!hoverMode.equals(DEFAULT_HOVER_MODE)) context.put("hoverMode", hoverMode);
     if (margin != null) {
@@ -300,7 +316,10 @@ public class Layout {
      * is initialized on each relayout. Note that, regardless of this attribute, an undefined layout
      * width or height is always initialized on the first call to plot.
      */
-    private final boolean autoSize = false;
+    private boolean autoSize = false;
+
+    private boolean widthSet = false;
+    private boolean heightSet = false;
 
     /** The width of the plot in pixels */
     private int width = 700;
@@ -409,11 +428,18 @@ public class Layout {
 
     public LayoutBuilder height(int height) {
       this.height = height;
+      this.heightSet = true;
       return this;
     }
 
     public LayoutBuilder width(int width) {
       this.width = width;
+      this.widthSet = true;
+      return this;
+    }
+
+    public LayoutBuilder autosize(boolean autosize) {
+      this.autoSize = autosize;
       return this;
     }
 
diff --git a/jsplot/src/main/resources/layout_template.html b/jsplot/src/main/resources/layout_template.html
index 8f9daa21..526b164d 100644
--- a/jsplot/src/main/resources/layout_template.html
+++ b/jsplot/src/main/resources/layout_template.html
@@ -5,8 +5,12 @@ var layout = {
 {% if titlefont is not null %}
     titlefont: {{titlefont | raw}},
 {% endif %}
+{% if height is not null %}
     height: {{height}},
+{% endif %}
+{% if width is not null %}
     width: {{width}},
+{% endif %}
 {% if showlegend is not null %}
     showlegend: {{showlegend}},
 {% endif %}


Test patch:
diff --git a/core/src/test/java/tech/tablesaw/aggregate/AggregateFunctionsTest.java b/core/src/test/java/tech/tablesaw/aggregate/AggregateFunctionsTest.java
index 598eec36..e7473608 100644
--- a/core/src/test/java/tech/tablesaw/aggregate/AggregateFunctionsTest.java
+++ b/core/src/test/java/tech/tablesaw/aggregate/AggregateFunctionsTest.java
@@ -41,6 +41,7 @@ import static tech.tablesaw.api.QuerySupport.date;
 import static tech.tablesaw.api.QuerySupport.num;
 import static tech.tablesaw.api.QuerySupport.str;
 
+import java.time.Instant;
 import java.time.LocalDate;
 import org.apache.commons.math3.stat.StatUtils;
 import org.junit.jupiter.api.BeforeEach;
@@ -48,6 +49,7 @@ import org.junit.jupiter.api.Test;
 import tech.tablesaw.api.BooleanColumn;
 import tech.tablesaw.api.DateColumn;
 import tech.tablesaw.api.DoubleColumn;
+import tech.tablesaw.api.InstantColumn;
 import tech.tablesaw.api.StringColumn;
 import tech.tablesaw.api.Table;
 import tech.tablesaw.io.csv.CsvReadOptions;
@@ -84,6 +86,66 @@ class AggregateFunctionsTest {
     assertEquals(13, result.rowCount());
   }
 
+  @Test
+  void testInstantMinMax() {
+    Instant i1 = Instant.ofEpochMilli(10_000L);
+    Instant i2 = Instant.ofEpochMilli(20_000L);
+    Instant i3 = Instant.ofEpochMilli(30_000L);
+    Instant i4 = null;
+
+    // Explicitly test having a first value of missing 
+    InstantColumn ic = InstantColumn.create("instants", 5);
+    ic.appendMissing();
+    ic.append(i3);
+    ic.append(i1);
+    ic.append(i2);
+    ic.appendMissing();
+    ic.append(i4);
+    Table test = Table.create("testInstantMath", ic);
+    Table minI = test.summarize("instants", AggregateFunctions.minInstant).apply();
+    Table maxI = test.summarize("instants", AggregateFunctions.maxInstant).apply();
+    assertEquals(i1, minI.get(0, 0));
+    assertEquals(i3, maxI.get(0, 0));
+  }
+
+  @Test
+  void testInstantMinWorksWithLeadingNull() {
+    Instant i1 = null;
+    Instant i2 = Instant.ofEpochMilli(20_000L);
+    Instant i3 = Instant.ofEpochMilli(30_000L);
+
+    InstantColumn ic1 = InstantColumn.create("instants", 3);
+    ic1.append(i1);
+    ic1.append(i2);
+    ic1.append(i3);
+    assertEquals(i2, ic1.min());
+
+    InstantColumn ic2 = InstantColumn.create("instants", 3);
+    ic2.appendMissing();
+    ic2.append(i2);
+    ic2.append(i3);
+    assertEquals(i2, ic2.min());
+  }
+
+  @Test
+  void testInstantMaxWorksWithLeadingNull() {
+    Instant i1 = null;
+    Instant i2 = Instant.ofEpochMilli(20_000L);
+    Instant i3 = Instant.ofEpochMilli(30_000L);
+
+    InstantColumn ic1 = InstantColumn.create("instants", 3);
+    ic1.append(i1);
+    ic1.append(i2);
+    ic1.append(i3);
+    assertEquals(i3, ic1.max());
+
+    InstantColumn ic2 = InstantColumn.create("instants", 3);
+    ic2.appendMissing();
+    ic2.append(i2);
+    ic2.append(i3);
+    assertEquals(i3, ic2.max());
+  }
+
   @Test
   void testHaving() {
     StringColumn byColumn = table.dateColumn("date").yearQuarter();
diff --git a/jsplot/src/test/java/tech/tablesaw/components/LayoutTest.java b/jsplot/src/test/java/tech/tablesaw/components/LayoutTest.java
index c92f856d..29a72bff 100644
--- a/jsplot/src/test/java/tech/tablesaw/components/LayoutTest.java
+++ b/jsplot/src/test/java/tech/tablesaw/components/LayoutTest.java
@@ -1,18 +1,17 @@
 package tech.tablesaw.components;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 import tech.tablesaw.plotly.components.Axis;
 import tech.tablesaw.plotly.components.Grid;
 import tech.tablesaw.plotly.components.Layout;
 import tech.tablesaw.plotly.components.Margin;
 
-@Disabled
 public class LayoutTest {
 
-  @Test
+  // @Test
   public void asJavascript() {
 
     Axis x = Axis.builder().title("x axis").build();
@@ -29,7 +28,7 @@ public class LayoutTest {
     System.out.println(layout.asJavascript());
   }
 
-  @Test
+  // @Test
   public void asJavascriptForGrid() {
 
     Axis x = Axis.builder().title("x axis").build();
@@ -50,4 +49,65 @@ public class LayoutTest {
     assertTrue(asJavascript.contains("rows"));
     assertTrue(asJavascript.contains("xAxis"));
   }
+
+  @Test
+  public void testAutosize() {
+    {
+      Layout layout = Layout.builder().autosize(true).build();
+      assertEquals(
+          "var layout = {\n"
+              + //
+              "    autosize: true,\n"
+              + //
+              "\n\n"
+              + //
+              "};\n",
+          layout.asJavascript());
+    }
+    {
+      Layout layout = Layout.builder().autosize(true).width(800).build();
+      assertEquals(
+          "var layout = {\n"
+              + //
+              "    width: 800,\n"
+              + //
+              "    autosize: true,\n"
+              + //
+              "\n\n"
+              + //
+              "};\n",
+          layout.asJavascript());
+    }
+    {
+      Layout layout = Layout.builder().autosize(true).height(600).width(800).build();
+      assertEquals(
+          "var layout = {\n"
+              + //
+              "    height: 600,\n"
+              + //
+              "    width: 800,\n"
+              + //
+              "    autosize: true,\n"
+              + //
+              "\n\n"
+              + //
+              "};\n",
+          layout.asJavascript());
+    }
+    {
+      // see if 700x450
+      Layout layout = Layout.builder().autosize(false).height(600).build();
+      assertEquals(
+          "var layout = {\n"
+              + //
+              "    height: 600,\n"
+              + //
+              "    width: 700,\n"
+              + //
+              "\n\n"
+              + //
+              "};\n",
+          layout.asJavascript());
+    }
+  }
 }