Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect sort order when hidden, pivoted columns are sorted #1000

Merged
merged 3 commits into from
Apr 6, 2020
Merged

Fix incorrect sort order when hidden, pivoted columns are sorted #1000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba0df04
fix issue with aggregates in hidden sorts when used as pivots
sc1f Apr 3, 2020
94a7b78
make aggregate for hidden column sorts in pivots AGGTYPE_UNIQUE, not …
sc1f Apr 3, 2020
4181917
column-only hidden sorts aggregate by ANY, hidden sorts that are in p…
sc1f Apr 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions cpp/perspective/src/cpp/view_config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,19 @@ t_view_config::fill_aggspecs(const t_schema& schema) {
= std::find(m_columns.begin(), m_columns.end(), column) == m_columns.end();

if (is_hidden_column) {
bool is_pivot = (std::find(m_row_pivots.begin(), m_row_pivots.end(), column)
!= m_row_pivots.end())
|| (std::find(m_column_pivots.begin(), m_column_pivots.end(), column)
!= m_column_pivots.end());
bool is_row_pivot = std::find(m_row_pivots.begin(), m_row_pivots.end(), column) != m_row_pivots.end();
bool is_column_pivot = std::find(m_column_pivots.begin(), m_column_pivots.end(), column) != m_column_pivots.end();
bool is_column_only = m_row_pivots.size() == 0 || m_column_only;

std::vector<t_dep> dependencies{t_dep(column, DEPTYPE_COLUMN)};
t_aggtype agg_type;

// use the `any` agg for columns used as pivots/column_only views
if (m_row_pivots.size() == 0 || m_column_only) {
agg_type = t_aggtype::AGGTYPE_ANY;
if (is_column_only) {
// Always sort by `ANY` in column only views
agg_type = t_aggtype::AGGTYPE_ANY;
} else if (is_row_pivot || is_column_pivot) {
// Otherwise if the hidden column is in pivots, use `UNIQUE`
agg_type = t_aggtype::AGGTYPE_UNIQUE;
} else if (m_aggregates.count(column) > 0) {
auto col = m_aggregates.at(column);
if (col.at(0) == "weighted mean") {
Expand All @@ -211,8 +213,6 @@ t_view_config::fill_aggspecs(const t_schema& schema) {
} else {
agg_type = str_to_aggtype(col.at(0));
}
} else if (is_pivot) {
agg_type = t_aggtype::AGGTYPE_ANY;
} else {
t_dtype dtype = schema.get_dtype(column);
agg_type = _get_default_aggregate(dtype);
Expand Down
221 changes: 185 additions & 36 deletions packages/perspective/test/js/sort.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,66 @@ module.exports = perspective => {
table.delete();
});

it("row pivot and hidden sort ['y'] with aggregates specified", async function() {
const table = perspective.table({
x: [1, 2, 3, 4, 5],
y: ["a", "b", "b", "b", "c"]
});
// Aggregate should be overriden if the sort column is hidden
// AND also in row pivots
const view = table.view({
aggregates: {
x: "sum",
y: "count"
},
columns: ["x"],
row_pivots: ["y"],
sort: [["y", "desc"]]
});
const answer = [
{__ROW_PATH__: [], x: 15},
{__ROW_PATH__: ["c"], x: 5},
{__ROW_PATH__: ["b"], x: 9},
{__ROW_PATH__: ["a"], x: 1}
];
const result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});

it("row pivot and hidden sort ['y'] without aggregates specified", async function() {
const table = perspective.table({
x: [1, 2, 3, 4, 5],
y: ["a", "b", "b", "b", "c"]
});
const view = table.view({
columns: ["x"],
row_pivots: ["y"],
sort: [["y", "desc"]]
});
const answer = [
{__ROW_PATH__: [], x: 15},
{__ROW_PATH__: ["c"], x: 5},
{__ROW_PATH__: ["b"], x: 9},
{__ROW_PATH__: ["a"], x: 1}
];
const result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});

it("column pivot ['y']", async function() {
var table = perspective.table(data);
var view = table.view({
const table = perspective.table(data);
const view = table.view({
columns: ["w"],
column_pivots: ["y"],
sort: [["x", "desc"]]
});
var answer = [
const paths = await view.column_paths();
expect(paths).toEqual(["a|w", "b|w", "c|w", "d|w"]);
const answer = [
{"a|w": null, "b|w": null, "c|w": null, "d|w": 4.5},
{"a|w": 5.5, "b|w": null, "c|w": null, "d|w": null},
{"a|w": null, "b|w": null, "c|w": 3.5, "d|w": null},
Expand All @@ -119,36 +171,106 @@ module.exports = perspective => {
{"a|w": 1.5, "b|w": null, "c|w": null, "d|w": null},
{"a|w": null, "b|w": null, "c|w": null, "d|w": 8.5}
];
let result = await view.to_json();
const result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});

it("column pivot ['y'] with overridden aggregates", async function() {
var table = perspective.table(data);
var view = table.view({
it("column pivot ['y'], col desc sort", async function() {
const table = perspective.table(data);
const view = table.view({
columns: ["w"],
column_pivots: ["y"],
aggregates: {x: "count"},
sort: [["x", "desc"]]
sort: [["x", "col desc"]]
});
var answer = [
{"a|w": null, "b|w": null, "c|w": null, "d|w": 4.5},
{"a|w": 5.5, "b|w": null, "c|w": null, "d|w": null},
{"a|w": null, "b|w": null, "c|w": 3.5, "d|w": null},
{"a|w": null, "b|w": 6.5, "c|w": null, "d|w": null},
{"a|w": null, "b|w": 2.5, "c|w": null, "d|w": null},
{"a|w": null, "b|w": null, "c|w": 7.5, "d|w": null},
{"a|w": 1.5, "b|w": null, "c|w": null, "d|w": null},
{"a|w": null, "b|w": null, "c|w": null, "d|w": 8.5}
];
let result = await view.to_json();
const paths = await view.column_paths();
expect(paths).toEqual(["d|w", "c|w", "b|w", "a|w"]);
const answer = {
"d|w": [null, null, null, 4.5, null, null, null, 8.5],
"c|w": [null, null, 3.5, null, null, null, 7.5, null],
"b|w": [null, 2.5, null, null, null, 6.5, null, null],
"a|w": [1.5, null, null, null, 5.5, null, null, null]
};
const result = await view.to_columns();
expect(result).toEqual(answer);
view.delete();
table.delete();
});

it("column pivot and hidden sort ['y']", async function() {
const table = perspective.table({
x: [1, 2, 3, 4],
y: ["a", "a", "a", "b"]
});
const view = table.view({
columns: ["x"],
column_pivots: ["y"],
sort: [["y", "desc"]]
});

const paths = await view.column_paths();
// regular non-col sort should not change order of column paths
expect(paths).toEqual(["a|x", "b|x"]);

const result = await view.to_columns();
expect(result).toEqual({
"a|x": [null, 1, 2, 3],
"b|x": [4, null, null, null]
});
view.delete();
table.delete();
});

it("column pivot and hidden col sort ['y']", async function() {
const table = perspective.table({
x: [1, 2, 3, 4],
y: ["a", "a", "a", "b"]
});
const view = table.view({
columns: ["x"],
column_pivots: ["y"],
sort: [["y", "col desc"]]
});

const paths = await view.column_paths();
expect(paths).toEqual(["b|x", "a|x"]);

const result = await view.to_columns();
expect(result).toEqual({
"b|x": [null, null, null, 4],
"a|x": [1, 2, 3, null]
});
view.delete();
table.delete();
});

it("column pivot ['y'] with overridden aggregates", async function() {
const table = perspective.table({
x: [1, 2, 3, 4],
y: ["a", "a", "a", "b"]
});
const view = table.view({
columns: ["x"],
column_pivots: ["y"],
aggregates: {y: "count"},
sort: [["y", "col desc"]]
});

const paths = await view.column_paths();
// Col sort will override aggregate
expect(paths).toEqual(["b|x", "a|x"]);

let result = await view.to_columns();
expect(result).toEqual({
"b|x": [null, null, null, 4],
"a|x": [1, 2, 3, null]
});

view.delete();
table.delete();
});

it("column pivot ['y'] with extra aggregates", async function() {
var table = perspective.table(data);
var view = table.view({
Expand All @@ -173,23 +295,50 @@ module.exports = perspective => {
table.delete();
});

it("row pivot ['y'], column pivot ['z']", async function() {
var table = perspective.table(data);
var view = table.view({
columns: ["w"],
row_pivots: ["y"],
column_pivots: ["z"],
sort: [["x", "desc"]]
it("row pivot ['x'], column pivot ['y'], both hidden and asc sorted", async function() {
const table = perspective.table({
x: ["a", "a", "b", "c"],
y: ["x", "x", "y", "x"],
z: [1, 2, 3, 4]
});
var answer = [
{__ROW_PATH__: [], "false|w": 22, "true|w": 18},
{__ROW_PATH__: ["a"], "false|w": null, "true|w": 7},
{__ROW_PATH__: ["b"], "false|w": 9, "true|w": null},
{__ROW_PATH__: ["c"], "false|w": null, "true|w": 11},
{__ROW_PATH__: ["d"], "false|w": 13, "true|w": null}
];
let result = await view.to_json();
expect(result).toEqual(answer);
const view = table.view({
columns: ["z"],
row_pivots: ["x"],
column_pivots: ["y"],
sort: [
["x", "asc"],
["y", "col asc"]
]
});
const paths = await view.column_paths();
expect(paths).toEqual(["__ROW_PATH__", "x|z", "y|z"]);
const expected = {__ROW_PATH__: [[], ["a"], ["b"], ["c"]], "x|z": [7, 3, null, 4], "y|z": [3, null, 3, null]};
const result = await view.to_columns();
expect(result).toEqual(expected);
view.delete();
table.delete();
});

it("row pivot ['x'], column pivot ['y'], both hidden and desc sorted", async function() {
const table = perspective.table({
x: ["a", "a", "b", "c"],
y: ["x", "x", "y", "x"],
z: [1, 2, 3, 4]
});
const view = table.view({
columns: ["z"],
row_pivots: ["x"],
column_pivots: ["y"],
sort: [
["x", "desc"],
["y", "col desc"]
]
});
const paths = await view.column_paths();
expect(paths).toEqual(["__ROW_PATH__", "y|z", "x|z"]);
const expected = {__ROW_PATH__: [[], ["c"], ["b"], ["a"]], "y|z": [3, null, 3, null], "x|z": [7, 4, null, 3]};
const result = await view.to_columns();
expect(result).toEqual(expected);
view.delete();
table.delete();
});
Expand Down
2 changes: 1 addition & 1 deletion python/perspective/examples/remote.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<script src="https://unpkg.com/@finos/perspective-viewer"></script>
<script src="https://unpkg.com/@finos/perspective-viewer-datagrid"></script>
<script src="https://unpkg.com/@finos/perspective-viewer-d3fc"></script>
<script src="https://unpkg.com/@finos/perspective"></script>>
<script src="https://unpkg.com/@finos/perspective"></script>

<link rel='stylesheet' href="https://unpkg.com/@finos/perspective-viewer/dist/umd/material.dark.css">

Expand Down