From daeb7ab6df57ba9a30822db7abcd95aa8ac331e6 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Wed, 5 Sep 2018 10:08:53 +0100 Subject: [PATCH] Refactor ScatterplotFacet for JSON serialization --- .../browsing/facets/ScatterplotFacet.java | 267 +++++++++++------- .../browsing/GetScatterplotCommand.java | 3 +- .../facets/ScatterplotFacetTests.java | 80 ++++++ 3 files changed, 243 insertions(+), 107 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/tests/browsing/facets/ScatterplotFacetTests.java diff --git a/main/src/com/google/refine/browsing/facets/ScatterplotFacet.java b/main/src/com/google/refine/browsing/facets/ScatterplotFacet.java index 2a58913b3..d8398db13 100644 --- a/main/src/com/google/refine/browsing/facets/ScatterplotFacet.java +++ b/main/src/com/google/refine/browsing/facets/ScatterplotFacet.java @@ -79,30 +79,121 @@ public class ScatterplotFacet implements Facet { /* * Configuration, from the client side */ - protected String name; // name of facet - - protected String expression_x; // expression to compute the x numeric value(s) per row - protected String expression_y; // expression to compute the y numeric value(s) per row - protected String columnName_x; // column to base the x expression on, if any - protected String columnName_y; // column to base the y expression on, if any + public static class ScatterplotFacetConfig implements FacetConfig { + protected String name; // name of facet - protected int size; - protected int dim_x; - protected int dim_y; - protected int rotation; - - protected double l; - protected double dot; - - protected String image; + protected String expression_x; // expression to compute the x numeric value(s) per row + protected String expression_y; // expression to compute the y numeric value(s) per row + protected String columnName_x; // column to base the x expression on, if any + protected String columnName_y; // column to base the y expression on, if any + + protected int size; + protected int dim_x; + protected int dim_y; + protected String rotation_str; + protected int rotation; - protected String color_str; - protected Color color; + protected double l; + protected double dot; - protected double from_x; // the numeric selection for the x axis, from 0 to 1 - protected double to_x; - protected double from_y; // the numeric selection for the y axis, from 0 to 1 - protected double to_y; + protected String color_str; + protected Color color; + + protected double from_x; // the numeric selection for the x axis, from 0 to 1 + protected double to_x; + protected double from_y; // the numeric selection for the y axis, from 0 to 1 + protected double to_y; + + protected boolean selected; // false if we're certain that all rows will match + // and there isn't any filtering to do + + @Override + public void write(JSONWriter writer, Properties options) + throws JSONException { + writer.object(); + + writer.key("type"); writer.value("scatterplot"); + writer.key(NAME); writer.value(name); + writer.key(X_COLUMN_NAME); writer.value(columnName_x); + writer.key(X_EXPRESSION); writer.value(expression_x); + writer.key(Y_COLUMN_NAME); writer.value(columnName_y); + writer.key(Y_EXPRESSION); writer.value(expression_y); + writer.key(SIZE); writer.value(size); + writer.key(DOT); writer.value(dot); + if(!rotation_str.isEmpty()) { + writer.key(ROTATION); writer.value(rotation_str); + } + writer.key(DIM_X); writer.value(dim_x == LIN ? "lin" : "log"); + writer.key(DIM_Y); writer.value(dim_y == LIN ? "lin" : "log"); + if(!"000000".equals(color_str)) { + writer.key(COLOR); writer.value(color_str); + } + writer.key(FROM_X); writer.value(from_x); + writer.key(TO_X); writer.value(to_x); + writer.key(FROM_Y); writer.value(from_y); + writer.key(TO_Y); writer.value(to_y); + + writer.endObject(); + + } + + @Override + public ScatterplotFacet apply(Project project) { + ScatterplotFacet facet = new ScatterplotFacet(); + facet.initializeFromConfig(this, project); + return facet; + } + + public void initializeFromJSON(JSONObject o) { + name = o.getString(NAME); + l = size = (o.has(SIZE)) ? o.getInt(SIZE) : 100; + dot = (o.has(DOT)) ? o.getInt(DOT) : 0.5d; + + dim_x = (o.has(DIM_X)) ? getAxisDim(o.getString(DIM_X)) : LIN; + if (o.has(FROM_X) && o.has(TO_X)) { + from_x = o.getDouble(FROM_X); + to_x = o.getDouble(TO_X); + selected = true; + } else { + from_x = 0; + to_x = 1; + } + + dim_y = (o.has(DIM_Y)) ? getAxisDim(o.getString(DIM_Y)) : LIN; + if (o.has(FROM_Y) && o.has(TO_Y)) { + from_y = o.getDouble(FROM_Y); + to_y = o.getDouble(TO_Y); + selected = true; + } else { + from_y = 0; + to_y = 1; + } + + rotation_str = (o.has(ROTATION) ? o.getString(ROTATION) : ""); + rotation = getRotation(rotation_str); + + color_str = (o.has(COLOR)) ? o.getString(COLOR) : "000000"; + color = new Color(Integer.parseInt(color_str,16)); + + columnName_x = o.getString(X_COLUMN_NAME); + expression_x = o.getString(X_EXPRESSION); + + columnName_y = o.getString(Y_COLUMN_NAME); + expression_y = o.getString(Y_EXPRESSION); + } + + public static int getRotation(String rotation) { + rotation = rotation.toLowerCase(); + if ("cw".equals(rotation) || "right".equals(rotation)) { + return ScatterplotFacet.ROTATE_CW; + } else if ("ccw".equals(rotation) || "left".equals(rotation)) { + return ScatterplotFacet.ROTATE_CCW; + } else { + return NO_ROTATION; + } + } + } + ScatterplotFacetConfig config; /* * Derived configuration data @@ -120,8 +211,8 @@ public class ScatterplotFacet implements Facet { protected double max_y; protected AffineTransform t; - protected boolean selected; // false if we're certain that all rows will match - // and there isn't any filtering to do + protected String image; + public static final String NAME = "name"; public static final String IMAGE = "image"; @@ -168,17 +259,17 @@ public class ScatterplotFacet implements Facet { writer.object(); - writer.key(NAME); writer.value(name); - writer.key(X_COLUMN_NAME); writer.value(columnName_x); - writer.key(X_EXPRESSION); writer.value(expression_x); - writer.key(Y_COLUMN_NAME); writer.value(columnName_y); - writer.key(Y_EXPRESSION); writer.value(expression_y); - writer.key(SIZE); writer.value(size); - writer.key(DOT); writer.value(dot); - writer.key(ROTATION); writer.value(rotation); - writer.key(DIM_X); writer.value(dim_x); - writer.key(DIM_Y); writer.value(dim_y); - writer.key(COLOR); writer.value(color_str); + writer.key(NAME); writer.value(config.name); + writer.key(X_COLUMN_NAME); writer.value(config.columnName_x); + writer.key(X_EXPRESSION); writer.value(config.expression_x); + writer.key(Y_COLUMN_NAME); writer.value(config.columnName_y); + writer.key(Y_EXPRESSION); writer.value(config.expression_y); + writer.key(SIZE); writer.value(config.size); + writer.key(DOT); writer.value(config.dot); + writer.key(ROTATION); writer.value(config.rotation); + writer.key(DIM_X); writer.value(config.dim_x); + writer.key(DIM_Y); writer.value(config.dim_y); + writer.key(COLOR); writer.value(config.color_str); if (IMAGE_URI) { writer.key(IMAGE); writer.value(image); @@ -188,8 +279,8 @@ public class ScatterplotFacet implements Facet { writer.key(ERROR_X); writer.value(errorMessage_x); } else { if (!Double.isInfinite(min_x) && !Double.isInfinite(max_x)) { - writer.key(FROM_X); writer.value(from_x); - writer.key(TO_X); writer.value(to_x); + writer.key(FROM_X); writer.value(config.from_x); + writer.key(TO_X); writer.value(config.to_x); } } @@ -197,8 +288,8 @@ public class ScatterplotFacet implements Facet { writer.key(ERROR_Y); writer.value(errorMessage_y); } else { if (!Double.isInfinite(min_y) && !Double.isInfinite(max_y)) { - writer.key(FROM_Y); writer.value(from_y); - writer.key(TO_Y); writer.value(to_y); + writer.key(FROM_Y); writer.value(config.from_y); + writer.key(TO_Y); writer.value(config.to_y); } } @@ -207,80 +298,54 @@ public class ScatterplotFacet implements Facet { @Override public void initializeFromJSON(Project project, JSONObject o) throws JSONException { - name = o.getString(NAME); - l = size = (o.has(SIZE)) ? o.getInt(SIZE) : 100; - dot = (o.has(DOT)) ? o.getInt(DOT) : 0.5d; + ScatterplotFacetConfig config = new ScatterplotFacetConfig(); + config.initializeFromJSON(o); + initializeFromConfig(config, project); + } + + public void initializeFromConfig(ScatterplotFacetConfig configuration, Project project) { + config = configuration; - dim_x = (o.has(DIM_X)) ? getAxisDim(o.getString(DIM_X)) : LIN; - if (o.has(FROM_X) && o.has(TO_X)) { - from_x = o.getDouble(FROM_X); - to_x = o.getDouble(TO_X); - selected = true; - } else { - from_x = 0; - to_x = 1; - } + t = createRotationMatrix(config.rotation, config.l); - dim_y = (o.has(DIM_Y)) ? getAxisDim(o.getString(DIM_Y)) : LIN; - if (o.has(FROM_Y) && o.has(TO_Y)) { - from_y = o.getDouble(FROM_Y); - to_y = o.getDouble(TO_Y); - selected = true; - } else { - from_y = 0; - to_y = 1; - } - - rotation = (o.has(ROTATION)) ? getRotation(o.getString(ROTATION)) : NO_ROTATION; - t = createRotationMatrix(rotation, l); - - color_str = (o.has(COLOR)) ? o.getString(COLOR) : "000000"; - color = new Color(Integer.parseInt(color_str,16)); - - columnName_x = o.getString(X_COLUMN_NAME); - expression_x = o.getString(X_EXPRESSION); - - if (columnName_x.length() > 0) { - Column x_column = project.columnModel.getColumnByName(columnName_x); + if (config.columnName_x.length() > 0) { + Column x_column = project.columnModel.getColumnByName(config.columnName_x); if (x_column != null) { columnIndex_x = x_column.getCellIndex(); - NumericBinIndex index_x = ScatterplotFacet.getBinIndex(project, x_column, eval_x, expression_x); + NumericBinIndex index_x = ScatterplotFacet.getBinIndex(project, x_column, eval_x, config.expression_x); min_x = index_x.getMin(); max_x = index_x.getMax(); } else { - errorMessage_x = "No column named " + columnName_x; + errorMessage_x = "No column named " + config.columnName_x; } } else { columnIndex_x = -1; } try { - eval_x = MetaParser.parse(expression_x); + eval_x = MetaParser.parse(config.expression_x); } catch (ParsingException e) { errorMessage_x = e.getMessage(); } - columnName_y = o.getString(Y_COLUMN_NAME); - expression_y = o.getString(Y_EXPRESSION); - - if (columnName_y.length() > 0) { - Column y_column = project.columnModel.getColumnByName(columnName_y); + if (config.columnName_y.length() > 0) { + Column y_column = project.columnModel.getColumnByName(config.columnName_y); if (y_column != null) { columnIndex_y = y_column.getCellIndex(); - NumericBinIndex index_y = ScatterplotFacet.getBinIndex(project, y_column, eval_y, expression_y); + NumericBinIndex index_y = ScatterplotFacet.getBinIndex(project, y_column, eval_y, config.expression_y); min_y = index_y.getMin(); max_y = index_y.getMax(); } else { - errorMessage_y = "No column named " + columnName_y; + errorMessage_y = "No column named " + config.columnName_y; } } else { columnIndex_y = -1; } try { - eval_y = MetaParser.parse(expression_y); + eval_y = MetaParser.parse(config.expression_y); } catch (ParsingException e) { errorMessage_y = e.getMessage(); } @@ -289,22 +354,22 @@ public class ScatterplotFacet implements Facet { @Override public RowFilter getRowFilter(Project project) { - if (selected && + if (config.selected && eval_x != null && errorMessage_x == null && eval_y != null && errorMessage_y == null) { return new DualExpressionsNumberComparisonRowFilter( - eval_x, columnName_x, columnIndex_x, eval_y, columnName_y, columnIndex_y) { + eval_x, config.columnName_x, columnIndex_x, eval_y, config.columnName_y, columnIndex_y) { - double from_x_pixels = from_x * l; - double to_x_pixels = to_x * l; - double from_y_pixels = from_y * l; - double to_y_pixels = to_y * l; + double from_x_pixels = config.from_x * config.l; + double to_x_pixels = config.to_x * config.l; + double from_y_pixels = config.from_y * config.l; + double to_y_pixels = config.to_y * config.l; @Override protected boolean checkValues(double x, double y) { Point2D.Double p = new Point2D.Double(x,y); - p = translateCoordinates(p, min_x, max_x, min_y, max_y, dim_x, dim_y, l, t); + p = translateCoordinates(p, min_x, max_x, min_y, max_y, config.dim_x, config.dim_y, config.l, t); return p.x >= from_x_pixels && p.x <= to_x_pixels && p.y >= from_y_pixels && p.y <= to_y_pixels; }; }; @@ -323,10 +388,10 @@ public class ScatterplotFacet implements Facet { public void computeChoices(Project project, FilteredRows filteredRows) { if (eval_x != null && eval_y != null && errorMessage_x == null && errorMessage_y == null) { Column column_x = project.columnModel.getColumnByCellIndex(columnIndex_x); - NumericBinIndex index_x = getBinIndex(project, column_x, eval_x, expression_x, "row-based"); + NumericBinIndex index_x = getBinIndex(project, column_x, eval_x, config.expression_x, "row-based"); Column column_y = project.columnModel.getColumnByCellIndex(columnIndex_y); - NumericBinIndex index_y = getBinIndex(project, column_y, eval_y, expression_y, "row-based"); + NumericBinIndex index_y = getBinIndex(project, column_y, eval_y, config.expression_y, "row-based"); retrieveDataFromBinIndices(index_x, index_y); @@ -334,7 +399,7 @@ public class ScatterplotFacet implements Facet { if (index_x.isNumeric() && index_y.isNumeric()) { ScatterplotDrawingRowVisitor drawer = new ScatterplotDrawingRowVisitor( columnIndex_x, columnIndex_y, min_x, max_x, min_y, max_y, - size, dim_x, dim_y, rotation, dot, color + config.size, config.dim_x, config.dim_y, config.rotation, config.dot, config.color ); filteredRows.accept(project, drawer); @@ -354,10 +419,10 @@ public class ScatterplotFacet implements Facet { public void computeChoices(Project project, FilteredRecords filteredRecords) { if (eval_x != null && eval_y != null && errorMessage_x == null && errorMessage_y == null) { Column column_x = project.columnModel.getColumnByCellIndex(columnIndex_x); - NumericBinIndex index_x = getBinIndex(project, column_x, eval_x, expression_x, "record-based"); + NumericBinIndex index_x = getBinIndex(project, column_x, eval_x, config.expression_x, "record-based"); Column column_y = project.columnModel.getColumnByCellIndex(columnIndex_y); - NumericBinIndex index_y = getBinIndex(project, column_y, eval_y, expression_y, "record-based"); + NumericBinIndex index_y = getBinIndex(project, column_y, eval_y, config.expression_y, "record-based"); retrieveDataFromBinIndices(index_x, index_y); @@ -365,7 +430,7 @@ public class ScatterplotFacet implements Facet { if (index_x.isNumeric() && index_y.isNumeric()) { ScatterplotDrawingRowVisitor drawer = new ScatterplotDrawingRowVisitor( columnIndex_x, columnIndex_y, min_x, max_x, min_y, max_y, - size, dim_x, dim_y, rotation, dot, color + config.size, config.dim_x, config.dim_y, config.rotation, config.dot, config.color ); filteredRecords.accept(project, drawer); @@ -401,17 +466,7 @@ public class ScatterplotFacet implements Facet { public static int getAxisDim(String type) { return ("log".equals(type.toLowerCase())) ? LOG : LIN; } - - public static int getRotation(String rotation) { - rotation = rotation.toLowerCase(); - if ("cw".equals(rotation) || "right".equals(rotation)) { - return ScatterplotFacet.ROTATE_CW; - } else if ("ccw".equals(rotation) || "left".equals(rotation)) { - return ScatterplotFacet.ROTATE_CCW; - } else { - return NO_ROTATION; - } - } + public static NumericBinIndex getBinIndex(Project project, Column column, Evaluable eval, String expression) { return getBinIndex(project, column, eval, expression, "row-based"); diff --git a/main/src/com/google/refine/commands/browsing/GetScatterplotCommand.java b/main/src/com/google/refine/commands/browsing/GetScatterplotCommand.java index 9a6cc8f04..d54bf2230 100644 --- a/main/src/com/google/refine/commands/browsing/GetScatterplotCommand.java +++ b/main/src/com/google/refine/commands/browsing/GetScatterplotCommand.java @@ -53,6 +53,7 @@ import com.google.refine.browsing.Engine; import com.google.refine.browsing.FilteredRows; import com.google.refine.browsing.facets.ScatterplotDrawingRowVisitor; import com.google.refine.browsing.facets.ScatterplotFacet; +import com.google.refine.browsing.facets.ScatterplotFacet.ScatterplotFacetConfig; import com.google.refine.browsing.util.NumericBinIndex; import com.google.refine.commands.Command; import com.google.refine.expr.Evaluable; @@ -114,7 +115,7 @@ public class GetScatterplotCommand extends Command { int dim_x = (o.has(ScatterplotFacet.DIM_X)) ? ScatterplotFacet.getAxisDim(o.getString(ScatterplotFacet.DIM_X)) : ScatterplotFacet.LIN; int dim_y = (o.has(ScatterplotFacet.DIM_Y)) ? ScatterplotFacet.getAxisDim(o.getString(ScatterplotFacet.DIM_Y)) : ScatterplotFacet.LIN; - int rotation = (o.has(ScatterplotFacet.ROTATION)) ? ScatterplotFacet.getRotation(o.getString(ScatterplotFacet.ROTATION)) : ScatterplotFacet.NO_ROTATION; + int rotation = (o.has(ScatterplotFacet.ROTATION)) ? ScatterplotFacetConfig.getRotation(o.getString(ScatterplotFacet.ROTATION)) : ScatterplotFacet.NO_ROTATION; String color_str = (o.has(ScatterplotFacet.COLOR)) ? o.getString(ScatterplotFacet.COLOR) : "000000"; Color color = new Color(Integer.parseInt(color_str,16)); diff --git a/main/tests/server/src/com/google/refine/tests/browsing/facets/ScatterplotFacetTests.java b/main/tests/server/src/com/google/refine/tests/browsing/facets/ScatterplotFacetTests.java new file mode 100644 index 000000000..6c0c0b3c0 --- /dev/null +++ b/main/tests/server/src/com/google/refine/tests/browsing/facets/ScatterplotFacetTests.java @@ -0,0 +1,80 @@ +package com.google.refine.tests.browsing.facets; + +import org.json.JSONObject; +import org.testng.annotations.Test; + +import com.google.refine.browsing.Engine; +import com.google.refine.browsing.facets.ScatterplotFacet; +import com.google.refine.browsing.facets.ScatterplotFacet.ScatterplotFacetConfig; +import com.google.refine.model.Cell; +import com.google.refine.model.Project; +import com.google.refine.tests.RefineTest; +import com.google.refine.tests.util.TestUtils; + +public class ScatterplotFacetTests extends RefineTest { + public static String configJson = "{\n" + + " \"to_x\": 1,\n" + + " \"to_y\": 1,\n" + + " \"dot\": 1,\n" + + " \"from_x\": 0.21333333333333335,\n" + + " \"l\": 150,\n" + + " \"type\": \"scatterplot\",\n" + + " \"from_y\": 0.26666666666666666,\n" + + " \"dim_y\": \"lin\",\n" + + " \"ex\": \"value\",\n" + + " \"dim_x\": \"lin\",\n" + + " \"ey\": \"value\",\n" + + " \"cx\": \"my column\",\n" + + " \"cy\": \"e\",\n" + + " \"name\": \"my column (x) vs. e (y)\"\n" + + " }"; + + public static String facetJson = "{" + + "\"name\":\"my column (x) vs. e (y)\"," + + "\"cx\":\"my column\"," + + "\"ex\":\"value\"," + + "\"cy\":\"e\"," + + "\"ey\":\"value\"," + + "\"l\":150," + + "\"dot\":1," + + "\"r\":0," + + "\"dim_x\":0," + + "\"dim_y\":0," + + "\"color\":\"000000\"," + + "\"from_x\":0.21333333333333335," + + "\"to_x\":1," + + "\"from_y\":0.26666666666666666," + + "\"to_y\":1" + + "}"; + + @Test + public void serializeScatterplotFacetConfig() { + ScatterplotFacetConfig config = new ScatterplotFacetConfig(); + config.initializeFromJSON(new JSONObject(configJson)); + TestUtils.isSerializedTo(config, configJson); + } + + @Test + public void serializeScatterplotFacet() { + Project project = createCSVProject("my column,e\n" + + "89.2,89.2\n" + + "-45.9,-45.9\n" + + "blah,blah\n" + + "0.4,0.4\n"); + Engine engine = new Engine(project); + project.rows.get(0).cells.set(0, new Cell(89.2, null)); + project.rows.get(0).cells.set(1, new Cell(89.2, null)); + project.rows.get(1).cells.set(0, new Cell(-45.9, null)); + project.rows.get(1).cells.set(1, new Cell(-45.9, null)); + project.rows.get(3).cells.set(0, new Cell(0.4, null)); + project.rows.get(3).cells.set(1, new Cell(0.4, null)); + + ScatterplotFacetConfig config = new ScatterplotFacetConfig(); + config.initializeFromJSON(new JSONObject(configJson)); + + ScatterplotFacet facet = config.apply(project); + facet.computeChoices(project, engine.getAllFilteredRows()); + + TestUtils.isSerializedTo(facet, facetJson); + } +}