Skip to content

Commit

Permalink
Merge pull request #1112 from finos/fix-data-slice
Browse files Browse the repository at this point in the history 
Explicitly floor start row/col and ceil end row/col
  • Loading branch information
@texodus
texodus authored Jul 26, 2020
2 parents b2b2921 + 8fb93df commit 39a74e1
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 10 deletions.
8 changes: 4 additions & 4 deletions packages/perspective/src/js/perspective.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,10 @@ export default function(Module) {
const end_col = Math.min(max_cols, (options.end_col !== undefined ? options.end_col + psp_offset : viewport.width ? start_col + viewport.width : max_cols) * (hidden + 1));

// Return the calculated values
options.start_row = start_row;
options.end_row = end_row;
options.start_col = start_col;
options.end_col = end_col;
options.start_row = Math.floor(start_row);
options.end_row = Math.ceil(end_row);
options.start_col = Math.floor(start_col);
options.end_col = Math.ceil(end_col);

return options;
};
Expand Down
125 changes: 124 additions & 1 deletion packages/perspective/test/js/to_format.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*
*/

var int_float_string_data = [
const int_float_string_data = [
{int: 1, float: 2.25, string: "a", datetime: new Date()},
{int: 2, float: 3.5, string: "b", datetime: new Date()},
{int: 3, float: 4.75, string: "c", datetime: new Date()},
Expand All @@ -31,6 +31,8 @@ module.exports = perspective => {
start_row: 5
});
expect(json).toEqual([]);
view.delete();
table.delete();
});

it("should filter out invalid start columns", async function() {
Expand All @@ -40,6 +42,8 @@ module.exports = perspective => {
start_col: 5
});
expect(json).toEqual([{}, {}, {}, {}]);
view.delete();
table.delete();
});

it("should respect start/end rows", async function() {
Expand All @@ -52,6 +56,8 @@ module.exports = perspective => {
let comparator = int_float_string_data[2];
comparator.datetime = +comparator.datetime;
expect(json[0]).toEqual(comparator);
view.delete();
table.delete();
});

it("should respect end rows when larger than data size", async function() {
Expand All @@ -67,6 +73,8 @@ module.exports = perspective => {
return x;
})
);
view.delete();
table.delete();
});

it("should respect start/end columns", async function() {
Expand All @@ -78,6 +86,87 @@ module.exports = perspective => {
});
let comparator = {string: int_float_string_data.map(d => d.string)};
expect(json).toEqual(comparator);
view.delete();
table.delete();
});

it("should floor float start rows", async function() {
let table = perspective.table(int_float_string_data);
let view = table.view();
let json = await view.to_json({
start_row: 1.5
});
expect(json).toEqual(
int_float_string_data.slice(1).map(x => {
x.datetime = +x.datetime;
return x;
})
);
view.delete();
table.delete();
});

it("should ceil float end rows", async function() {
let table = perspective.table(int_float_string_data);
let view = table.view();
let json = await view.to_json({
end_row: 1.5
});
expect(json).toEqual(
// deep copy
JSON.parse(JSON.stringify(int_float_string_data))
.slice(0, 2)
.map(x => {
x.datetime = +new Date(x.datetime);
return x;
})
);
view.delete();
table.delete();
});

it("should floor/ceil float start/end rows", async function() {
let table = perspective.table(int_float_string_data);
let view = table.view();
let json = await view.to_json({
start_row: 2.9,
end_row: 2.4
});
let comparator = int_float_string_data[2];
comparator.datetime = +comparator.datetime;
expect(json[0]).toEqual(comparator);
view.delete();
table.delete();
});

it("should ceil float end rows when larger than data size", async function() {
let table = perspective.table(int_float_string_data);
let view = table.view();
let json = await view.to_json({
start_row: 2,
end_row: 5.5
});
expect(json).toEqual(
int_float_string_data.slice(2).map(x => {
x.datetime = +x.datetime;
return x;
})
);
view.delete();
table.delete();
});

it("should floor/ceil float start/end columns", async function() {
let table = perspective.table(int_float_string_data);
let view = table.view();
let json = await view.to_columns({
start_col: 2.6,
end_col: 2.4
});
let comparator = {string: int_float_string_data.map(d => d.string)};
expect(json).toEqual(comparator);
view.delete();
table.delete();
});

it("one-sided views should have row paths", async function() {
Expand All @@ -89,6 +178,8 @@ module.exports = perspective => {
for (let d of json) {
expect(d.__ROW_PATH__).toBeDefined();
}
view.delete();
table.delete();
});

it("one-sided views with start_col > 0 should have row paths", async function() {
Expand All @@ -100,6 +191,8 @@ module.exports = perspective => {
for (let d of json) {
expect(d.__ROW_PATH__).toBeDefined();
}
view.delete();
table.delete();
});

it("one-sided column-only views should not have row paths", async function() {
Expand All @@ -111,6 +204,8 @@ module.exports = perspective => {
for (let d of json) {
expect(d.__ROW_PATH__).toBeUndefined();
}
view.delete();
table.delete();
});

it("column-only views should not have header rows", async function() {
Expand All @@ -126,6 +221,8 @@ module.exports = perspective => {
{"1|x": 1, "1|y": "a", "2|x": null, "2|y": null},
{"1|x": null, "1|y": null, "2|x": 2, "2|y": "b"}
]);
view.delete();
table.delete();
});

it("column-only views should return correct windows of data", async function() {
Expand All @@ -140,6 +237,8 @@ module.exports = perspective => {
start_row: 1
});
expect(json).toEqual([{"1|x": null, "1|y": null, "2|x": 2, "2|y": "b"}]);
view.delete();
table.delete();
});

it("two-sided views should have row paths", async function() {
Expand All @@ -152,6 +251,8 @@ module.exports = perspective => {
for (let d of json) {
expect(d.__ROW_PATH__).toBeDefined();
}
view.delete();
table.delete();
});

it("two-sided views with start_col > 0 should have row paths", async function() {
Expand All @@ -164,6 +265,8 @@ module.exports = perspective => {
for (let d of json) {
expect(d.__ROW_PATH__).toBeDefined();
}
view.delete();
table.delete();
});

it("two-sided sorted views with start_col > 0 should have row paths", async function() {
Expand All @@ -177,6 +280,8 @@ module.exports = perspective => {
for (let d of json) {
expect(d.__ROW_PATH__).toBeDefined();
}
view.delete();
table.delete();
});
});

Expand All @@ -193,6 +298,8 @@ module.exports = perspective => {
let name = Object.keys(json[0])[1];
// make sure that number of separators = num of column pivots
expect((name.match(/\|/g) || []).length).toEqual(2);
view.delete();
table.delete();
});

it("should return dates in native form by default", async function() {
Expand Down Expand Up @@ -227,6 +334,8 @@ module.exports = perspective => {
{__ROW_PATH__: [3], datetime: 1, float: 4.75, int: 3, string: 1},
{__ROW_PATH__: [4], datetime: 1, float: 5.25, int: 4, string: 1}
]);
view.delete();
table.delete();
});
});

Expand Down Expand Up @@ -570,6 +679,8 @@ module.exports = perspective => {
index: true
});
expect(json).toEqual([{float: 2.25, __INDEX__: [0]}]);
view.delete();
table.delete();
});

it("should return correct pkey for float indexed table", async function() {
Expand All @@ -583,6 +694,8 @@ module.exports = perspective => {
index: true
});
expect(json).toEqual([{float: 2.25, __INDEX__: [2.25]}]);
view.delete();
table.delete();
});

it("should return correct pkey for string indexed table", async function() {
Expand All @@ -596,6 +709,8 @@ module.exports = perspective => {
index: true
});
expect(json).toEqual([{float: 2.25, __INDEX__: ["a"]}]);
view.delete();
table.delete();
});

it("should return correct pkey for date indexed table", async function() {
Expand All @@ -613,6 +728,8 @@ module.exports = perspective => {
index: true
});
expect(json).toEqual([{int: 2, datetime: data[1].datetime.getTime(), __INDEX__: [data[1].datetime.getTime()]}]);
view.delete();
table.delete();
});

it("should return correct pkey for all rows + columns on an unindexed table", async function() {
Expand All @@ -625,6 +742,8 @@ module.exports = perspective => {
for (let i = 0; i < json.length; i++) {
expect(json[i].__INDEX__).toEqual([i]);
}
view.delete();
table.delete();
});

it("should return correct pkey for all rows + columns on an indexed table", async function() {
Expand All @@ -638,6 +757,8 @@ module.exports = perspective => {
for (let i = 0; i < json.length; i++) {
expect(json[i].__INDEX__).toEqual([pkeys[i]]);
}
view.delete();
table.delete();
});
});
});
Expand All @@ -654,6 +775,8 @@ module.exports = perspective => {

// total row contains all pkeys for underlying rows; each aggregated row should have pkeys for the rows that belong to it
expect(json).toEqual(pivoted_output);
view.delete();
table.delete();
});
});

Expand Down
10 changes: 5 additions & 5 deletions python/perspective/perspective/table/_data_formatter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#

import numpy as np
from math import trunc
from math import floor, ceil, trunc
from ._constants import COLUMN_SEPARATOR_STRING
from .libbinding import get_data_slice_zero, get_data_slice_one, get_data_slice_two, \
get_from_data_slice_zero, get_from_data_slice_one, get_from_data_slice_two, \
Expand Down Expand Up @@ -137,10 +137,10 @@ def _parse_format_options(view, options):
'''Given a user-provided options dictionary, extract the useful values.'''
max_cols = view.num_columns() + (1 if view._sides > 0 else 0)
return {
"start_row": max(options.get("start_row", 0), 0),
"end_row": min(options.get("end_row", view.num_rows()), view.num_rows()),
"start_col": max(options.get("start_col", 0), 0),
"end_col": min(options.get("end_col", max_cols) * (view._num_hidden_cols() + 1), max_cols),
"start_row": int(floor(max(options.get("start_row", 0), 0))),
"end_row": int(ceil(min(options.get("end_row", view.num_rows()), view.num_rows()))),
"start_col": int(floor(max(options.get("start_col", 0), 0))),
"end_col": int(ceil(min(options.get("end_col", max_cols) * (view._num_hidden_cols() + 1), max_cols))),
"index": options.get("index", False),
"leaves_only": options.get("leaves_only", False),
"has_row_path": view._sides > 0 and (not view._column_only)
Expand Down
Loading

0 comments on commit 39a74e1

Please sign in to comment.