Skip to content

Commit

Permalink
Merge pull request #1623 from finos/partial-update-py
Browse files Browse the repository at this point in the history
Fix partial-updates on tables with `limit` set for python
  • Loading branch information
texodus authored Nov 22, 2021
2 parents 91f447a + 3038c5b commit bec0eb8
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 23 deletions.
43 changes: 22 additions & 21 deletions python/perspective/perspective/src/fill.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ namespace binding {

void
_fill_col_time(t_data_accessor accessor, std::shared_ptr<t_column> col, std::string name,
std::int32_t cidx, t_dtype type, bool is_update) {
std::int32_t cidx, t_dtype type, bool is_update, bool is_limit) {
t_uindex nrows = col->size();

for (auto i = 0; i < nrows; ++i) {
if (!accessor.attr("_has_column")(i, name).cast<bool>()) {
if (!accessor.attr("_has_column")(i, name).cast<bool>() && !is_limit) {
continue;
}

Expand All @@ -49,11 +49,11 @@ _fill_col_time(t_data_accessor accessor, std::shared_ptr<t_column> col, std::str

void
_fill_col_date(t_data_accessor accessor, std::shared_ptr<t_column> col, std::string name,
std::int32_t cidx, t_dtype type, bool is_update) {
std::int32_t cidx, t_dtype type, bool is_update, bool is_limit) {
t_uindex nrows = col->size();

for (auto i = 0; i < nrows; ++i) {
if (!accessor.attr("_has_column")(i, name).cast<bool>()) {
if (!accessor.attr("_has_column")(i, name).cast<bool>() && !is_limit) {
continue;
}

Expand All @@ -77,11 +77,11 @@ _fill_col_date(t_data_accessor accessor, std::shared_ptr<t_column> col, std::str

void
_fill_col_bool(t_data_accessor accessor, std::shared_ptr<t_column> col, std::string name,
std::int32_t cidx, t_dtype type, bool is_update) {
std::int32_t cidx, t_dtype type, bool is_update, bool is_limit) {
t_uindex nrows = col->size();

for (auto i = 0; i < nrows; ++i) {
if (!accessor.attr("_has_column")(i, name).cast<bool>()) {
if (!accessor.attr("_has_column")(i, name).cast<bool>() && !is_limit) {
continue;
}

Expand All @@ -103,12 +103,12 @@ _fill_col_bool(t_data_accessor accessor, std::shared_ptr<t_column> col, std::str

void
_fill_col_string(t_data_accessor accessor, std::shared_ptr<t_column> col, std::string name,
std::int32_t cidx, t_dtype type, bool is_update) {
std::int32_t cidx, t_dtype type, bool is_update, bool is_limit) {

t_uindex nrows = col->size();

for (auto i = 0; i < nrows; ++i) {
if (!accessor.attr("_has_column")(i, name).cast<bool>()) {
if (!accessor.attr("_has_column")(i, name).cast<bool>() && !is_limit) {
continue;
}

Expand Down Expand Up @@ -192,11 +192,11 @@ set_column_nth(std::shared_ptr<t_column> col, t_uindex idx, t_val value) {

void
_fill_col_numeric(t_data_accessor accessor, t_data_table& tbl,
std::shared_ptr<t_column> col, std::string name, std::int32_t cidx, t_dtype type, bool is_update) {
std::shared_ptr<t_column> col, std::string name, std::int32_t cidx, t_dtype type, bool is_update, bool is_limit) {
t_uindex nrows = col->size();

for (auto i = 0; i < nrows; ++i) {
if (!accessor.attr("_has_column")(i, name).cast<bool>()) {
if (!accessor.attr("_has_column")(i, name).cast<bool>() && !is_limit) {
continue;
}

Expand Down Expand Up @@ -260,7 +260,7 @@ _fill_col_numeric(t_data_accessor accessor, t_data_table& tbl,
tbl.promote_column(name, DTYPE_STR, i, false);
col = tbl.get_column(name);
_fill_col_string(
accessor, col, name, cidx, DTYPE_STR, is_update);
accessor, col, name, cidx, DTYPE_STR, is_update, is_limit);
return;
} else {
col->set_nth(i, static_cast<std::int32_t>(fval));
Expand All @@ -286,7 +286,7 @@ _fill_col_numeric(t_data_accessor accessor, t_data_table& tbl,
tbl.promote_column(name, DTYPE_STR, i, false);
col = tbl.get_column(name);
_fill_col_string(
accessor, col, name, cidx, DTYPE_STR, is_update);
accessor, col, name, cidx, DTYPE_STR, is_update, is_limit);
return;
} else {
col->set_nth(i, static_cast<std::int64_t>(fval));
Expand All @@ -308,7 +308,7 @@ _fill_col_numeric(t_data_accessor accessor, t_data_table& tbl,
tbl.promote_column(name, DTYPE_STR, i, false);
col = tbl.get_column(name);
_fill_col_string(
accessor, col, name, cidx, DTYPE_STR, is_update);
accessor, col, name, cidx, DTYPE_STR, is_update, is_limit);
return;
}

Expand Down Expand Up @@ -339,26 +339,26 @@ _fill_col_numeric(t_data_accessor accessor, t_data_table& tbl,

void
_fill_data_helper(t_data_accessor accessor, t_data_table& tbl,
std::shared_ptr<t_column> col, std::string name, std::int32_t cidx, t_dtype type, bool is_update) {
std::shared_ptr<t_column> col, std::string name, std::int32_t cidx, t_dtype type, bool is_update, bool is_limit) {
switch (type) {
case DTYPE_BOOL: {
_fill_col_bool(accessor, col, name, cidx, type, is_update);
_fill_col_bool(accessor, col, name, cidx, type, is_update, is_limit);
} break;
case DTYPE_DATE: {
_fill_col_date(accessor, col, name, cidx, type, is_update);
_fill_col_date(accessor, col, name, cidx, type, is_update, is_limit);
} break;
case DTYPE_TIME: {
_fill_col_time(accessor, col, name, cidx, type, is_update);
_fill_col_time(accessor, col, name, cidx, type, is_update, is_limit);
} break;
case DTYPE_STR: {
_fill_col_string(accessor, col, name, cidx, type, is_update);
_fill_col_string(accessor, col, name, cidx, type, is_update, is_limit);
} break;
case DTYPE_NONE: {
break;
}
default:
_fill_col_numeric(
accessor, tbl, col, name, cidx, type, is_update);
accessor, tbl, col, name, cidx, type, is_update, is_limit);
}
}

Expand All @@ -371,6 +371,7 @@ void
_fill_data(t_data_table& tbl, t_data_accessor accessor, const t_schema& input_schema,
const std::string& index, std::uint32_t offset, std::uint32_t limit, bool is_update) {
bool implicit_index = false;
bool is_limit = limit != UINT32_MAX;
std::vector<std::string> col_names(input_schema.columns());
std::vector<t_dtype> data_types(input_schema.types());

Expand All @@ -381,13 +382,13 @@ _fill_data(t_data_table& tbl, t_data_accessor accessor, const t_schema& input_sc
if (name == "__INDEX__") {
implicit_index = true;
std::shared_ptr<t_column> pkey_col_sptr = tbl.add_column_sptr("psp_pkey", type, true);
_fill_data_helper(accessor, tbl, pkey_col_sptr, "psp_pkey", cidx, type, is_update);
_fill_data_helper(accessor, tbl, pkey_col_sptr, "psp_pkey", cidx, type, is_update, is_limit);
tbl.clone_column("psp_pkey", "psp_okey");
continue;
}

auto col = tbl.get_column(name);
_fill_data_helper(accessor, tbl, col, name, cidx, type, is_update);
_fill_data_helper(accessor, tbl, col, name, cidx, type, is_update, is_limit);
}
// Fill index column - recreated every time a `t_data_table` is created.
if (!implicit_index) {
Expand Down
27 changes: 27 additions & 0 deletions python/perspective/perspective/tests/table/test_table_limit.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# *****************************************************************************
#
# Copyright (c) 2019, the Perspective Authors.
#
# This file is part of the Perspective library, distributed under the terms of
# the Apache License 2.0. The full license can be found in the LICENSE file.
#

import perspective
import six

from pytest import mark
from datetime import date, datetime


class TestTableInfer(object):

def test_table_limit_wraparound_does_not_respect_partial(self):
t = perspective.Table({'a':float, 'b':float}, limit=3)
t.update([{'a':10}, {'b':1}, {'a':20}, {'a':None,'b':2}])
df = t.view().to_df()

t2 = perspective.Table({'a':float, 'b':float}, limit=3)
t2.update([{'a':10}, {'b':1}, {'a':20}, {'b':2}])
df2 = t2.view().to_df()

assert df.to_dict() == df2.to_dict()
2 changes: 0 additions & 2 deletions rust/perspective-viewer/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ const BUILD = [
},
{
entryPoints: ["src/ts/perspective-viewer.ts"],
// metafile: false,
plugins: [
IgnoreCSSPlugin(),
IgnoreFontsPlugin(),
Expand All @@ -66,7 +65,6 @@ const BUILD = [
{
entryPoints: ["src/ts/perspective-viewer.ts"],
format: "esm",
metafile: false,
plugins: [
IgnoreCSSPlugin(),
IgnoreFontsPlugin(),
Expand Down

0 comments on commit bec0eb8

Please sign in to comment.