From 1e736e13cf0337198c74b4816b9796eee2d48378 Mon Sep 17 00:00:00 2001 From: Oleksiy Kononenko Date: Wed, 1 Jun 2022 17:09:46 -0700 Subject: [PATCH 1/6] Add support for void columns for all the row-functions --- src/core/expr/fnary/fnary.cc | 19 ++++++--- src/core/expr/fnary/fnary.h | 46 ++++++++++++++++----- src/core/expr/fnary/rowall.cc | 18 ++++---- src/core/expr/fnary/rowany.cc | 27 +++++------- src/core/expr/fnary/rowcount.cc | 8 ++-- src/core/expr/fnary/rowfirstlast.cc | 38 +++++++---------- src/core/expr/fnary/rowmean.cc | 7 +++- src/core/expr/fnary/rowminmax.cc | 12 +++++- src/core/expr/fnary/rowsd.cc | 7 +++- src/core/expr/fnary/rowsum.cc | 6 ++- tests/ijby/test-rowwise.py | 64 +++++++++++++++++++++++++++-- 11 files changed, 174 insertions(+), 78 deletions(-) diff --git a/src/core/expr/fnary/fnary.cc b/src/core/expr/fnary/fnary.cc index 52f68fa3c2..663afbb396 100644 --- a/src/core/expr/fnary/fnary.cc +++ b/src/core/expr/fnary/fnary.cc @@ -30,8 +30,8 @@ namespace dt { namespace expr { -FExpr_RowFn::FExpr_RowFn(ptrExpr&& args) - : args_(std::move(args)) +FExpr_RowFn::FExpr_RowFn(ptrExpr&& args, bool process_void_cols /* =false */) + : args_(std::move(args)), process_void_cols_(process_void_cols) {} @@ -47,15 +47,21 @@ std::string FExpr_RowFn::repr() const { Workframe FExpr_RowFn::evaluate_n(EvalContext& ctx) const { Workframe inputs = args_->evaluate_n(ctx); Grouping gmode = inputs.get_grouping_mode(); - std::vector columns; - columns.reserve(inputs.ncols()); + colvec columns; + size_t ncols = inputs.ncols(); + size_t nrows = 1; + columns.reserve(ncols); for (size_t i = 0; i < inputs.ncols(); ++i) { - columns.emplace_back(inputs.retrieve_column(i)); + Column col = inputs.retrieve_column(i); + nrows = col.nrows(); + if (process_void_cols_ || !col.type().is_void()) { + columns.emplace_back(col); + } } Workframe out(ctx); out.add_column( - apply_function(std::move(columns)), + apply_function(std::move(columns), nrows, ncols), "", gmode ); return out; @@ -66,6 +72,7 @@ SType FExpr_RowFn::common_numeric_stype(const colvec& columns) const { SType common_stype = SType::INT32; for (size_t i = 0; i < columns.size(); ++i) { switch (columns[i].stype()) { + case SType::VOID: case SType::BOOL: case SType::INT8: case SType::INT16: diff --git a/src/core/expr/fnary/fnary.h b/src/core/expr/fnary/fnary.h index 85b4aea4fc..aecf12f36d 100644 --- a/src/core/expr/fnary/fnary.h +++ b/src/core/expr/fnary/fnary.h @@ -51,14 +51,23 @@ py::oobj py_rowfn(const py::XArgs& args); class FExpr_RowFn : public FExpr_Func { private: ptrExpr args_; + bool process_void_cols_; + size_t : 56; public: - FExpr_RowFn(ptrExpr&& args); + FExpr_RowFn(ptrExpr&& args, bool process_void_cols = false); std::string repr() const override; Workframe evaluate_n(EvalContext& ctx) const override; virtual std::string name() const = 0; - virtual Column apply_function(std::vector&& columns) const = 0; + virtual Column apply_function( + colvec&& columns, // columns to process; if `process_void_cols_` is `False` + // void columns are filtered out + const size_t nrows, // number of rows in the original input frame; needed in the case + // when all the columns are void and filtered out + const size_t ncols // number of columns in the original input frame, + // including the void columns + ) const = 0; SType common_numeric_stype(const colvec&) const; void promote_columns(colvec& columns, SType target_stype) const; @@ -71,7 +80,9 @@ class FExpr_RowAll : public FExpr_RowFn { using FExpr_RowFn::FExpr_RowFn; std::string name() const override; - Column apply_function(std::vector&& columns) const override; + Column apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const override; }; @@ -81,7 +92,9 @@ class FExpr_RowAny : public FExpr_RowFn { using FExpr_RowFn::FExpr_RowFn; std::string name() const override; - Column apply_function(std::vector&& columns) const override; + Column apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const override; }; @@ -91,7 +104,9 @@ class FExpr_RowCount : public FExpr_RowFn { using FExpr_RowFn::FExpr_RowFn; std::string name() const override; - Column apply_function(std::vector&& columns) const override; + Column apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const override; }; @@ -102,7 +117,9 @@ class FExpr_RowFirstLast : public FExpr_RowFn { using FExpr_RowFn::FExpr_RowFn; std::string name() const override; - Column apply_function(std::vector&& columns) const override; + Column apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const override; }; extern template class FExpr_RowFirstLast; @@ -115,8 +132,11 @@ class FExpr_RowMinMax : public FExpr_RowFn { public: using FExpr_RowFn::FExpr_RowFn; + FExpr_RowMinMax(ptrExpr&& args); std::string name() const override; - Column apply_function(std::vector&& columns) const override; + Column apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const override; }; extern template class FExpr_RowMinMax; @@ -131,7 +151,9 @@ class FExpr_RowMean : public FExpr_RowFn { using FExpr_RowFn::FExpr_RowFn; std::string name() const override; - Column apply_function(std::vector&& columns) const override; + Column apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const override; }; @@ -141,7 +163,9 @@ class FExpr_RowSd : public FExpr_RowFn { using FExpr_RowFn::FExpr_RowFn; std::string name() const override; - Column apply_function(std::vector&& columns) const override; + Column apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const override; }; @@ -151,7 +175,9 @@ class FExpr_RowSum : public FExpr_RowFn { using FExpr_RowFn::FExpr_RowFn; std::string name() const override; - Column apply_function(std::vector&& columns) const override; + Column apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const override; }; diff --git a/src/core/expr/fnary/rowall.cc b/src/core/expr/fnary/rowall.cc index 3e19b22268..435bfec581 100644 --- a/src/core/expr/fnary/rowall.cc +++ b/src/core/expr/fnary/rowall.cc @@ -49,17 +49,20 @@ static bool op_rowall(size_t i, int8_t* out, const colvec& columns) { } -Column FExpr_RowAll::apply_function(colvec&& columns) const { - if (columns.empty()) { +Column FExpr_RowAll::apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const { + // No columns + if (ncols == 0) { return Const_ColumnImpl::make_bool_column(1, true); } - size_t nrows = columns[0].nrows(); + // Some void columns + if (columns.size() != ncols) { + return Const_ColumnImpl::make_bool_column(nrows, false); + } + // No void columns for (size_t i = 0; i < columns.size(); ++i) { xassert(columns[i].nrows() == nrows); - // If there is even one void column, the result of `rowall()` is `false` - if (columns[i].type().is_void()) { - return Const_ColumnImpl::make_bool_column(nrows, false); - } if (!columns[i].type().is_boolean()) { throw TypeError() << "Function `rowall` requires a sequence of boolean " "columns, however column " << i << " has type `" @@ -70,6 +73,7 @@ Column FExpr_RowAll::apply_function(colvec&& columns) const { std::move(columns), op_rowall, nrows, SType::BOOL)); } + DECLARE_PYFN(&py_rowfn) ->name("rowall") ->docs(dt::doc_dt_rowall) diff --git a/src/core/expr/fnary/rowany.cc b/src/core/expr/fnary/rowany.cc index 29b0e8c9f1..0fa6cccffd 100644 --- a/src/core/expr/fnary/rowany.cc +++ b/src/core/expr/fnary/rowany.cc @@ -50,29 +50,24 @@ static bool op_rowany(size_t i, int8_t* out, const colvec& columns) { -Column FExpr_RowAny::apply_function(colvec&& columns) const { - size_t ncols = columns.size(); - size_t nrows = ncols? columns[0].nrows() : 1; - colvec columns_; - columns_.reserve(ncols); - - for (size_t i = 0; i < ncols; ++i) { +Column FExpr_RowAny::apply_function(colvec&& columns, + const size_t nrows, + const size_t ncols) const { + // No non-void columns + if (columns.empty()) { + // `ncols == 0` tests that there is no void columns + return Const_ColumnImpl::make_bool_column(nrows, ncols == 0); + } + for (size_t i = 0; i < columns.size(); ++i) { xassert(columns[i].nrows() == nrows); - if (!columns[i].type().is_boolean_or_void()) { + if (!columns[i].type().is_boolean()) { throw TypeError() << "Function `rowany` requires a sequence of boolean " "columns, however column " << i << " has type `" << columns[i].stype() << "`"; } - // Filter out void columns, since they don't affect result of `rowany()` - if (columns[i].type().is_boolean()) { - columns_.push_back(std::move(columns[i])); - } - } - if (columns_.empty()) { - return Const_ColumnImpl::make_bool_column(nrows, columns.empty()); } return Column(new FuncNary_ColumnImpl( - std::move(columns_), op_rowany, nrows, SType::BOOL)); + std::move(columns), op_rowany, nrows, SType::BOOL)); } diff --git a/src/core/expr/fnary/rowcount.cc b/src/core/expr/fnary/rowcount.cc index 23f9a47018..a750a7f69a 100644 --- a/src/core/expr/fnary/rowcount.cc +++ b/src/core/expr/fnary/rowcount.cc @@ -50,11 +50,13 @@ static bool op_rowcount(size_t i, int32_t* out, const colvec& columns) { } -Column FExpr_RowCount::apply_function(colvec&& columns) const { +Column FExpr_RowCount::apply_function(colvec&& columns, + const size_t nrows, + const size_t) const +{ if (columns.empty()) { - return Const_ColumnImpl::make_int_column(1, 0, SType::INT32); + return Const_ColumnImpl::make_int_column(nrows, 0, SType::INT32); } - size_t nrows = columns[0].nrows(); for (size_t i = 0; i < columns.size(); ++i) { xassert(columns[i].nrows() == nrows); columns[i] = unaryop(Op::ISNA, std::move(columns[i])); diff --git a/src/core/expr/fnary/rowfirstlast.cc b/src/core/expr/fnary/rowfirstlast.cc index fb33d69968..b532a6b32f 100644 --- a/src/core/expr/fnary/rowfirstlast.cc +++ b/src/core/expr/fnary/rowfirstlast.cc @@ -57,43 +57,33 @@ static inline Column _rowfirstlast(colvec&& columns, SType outtype) { template -Column FExpr_RowFirstLast::apply_function(colvec&& columns) const { - size_t ncols = columns.size(); - size_t nrows = ncols? columns[0].nrows() : 1; - colvec columns_; - columns_.reserve(ncols); - - for (size_t i = 0; i < ncols; ++i) { - // Filter out void columns, since they don't affect the result - if (!columns[i].type().is_void()) { - columns_.push_back(std::move(columns[i])); - } - } - - if (columns_.empty()) { +Column FExpr_RowFirstLast::apply_function(colvec&& columns, + const size_t nrows, + const size_t) const { + if (columns.empty()) { return Const_ColumnImpl::make_na_column(nrows); } // Detect common stype SType stype0 = SType::VOID; - for (const auto& col : columns_) { + for (const auto& col : columns) { stype0 = common_stype(stype0, col.stype()); } if (stype0 == SType::INVALID) { throw TypeError() << "Incompatible column types in function `" << name() << "`"; } - promote_columns(columns_, stype0); + promote_columns(columns, stype0); switch (stype0) { - case SType::BOOL: return _rowfirstlast(std::move(columns_), stype0); - case SType::INT8: return _rowfirstlast(std::move(columns_), stype0); - case SType::INT16: return _rowfirstlast(std::move(columns_), stype0); - case SType::INT32: return _rowfirstlast(std::move(columns_), stype0); - case SType::INT64: return _rowfirstlast(std::move(columns_), stype0); - case SType::FLOAT32: return _rowfirstlast(std::move(columns_), stype0); - case SType::FLOAT64: return _rowfirstlast(std::move(columns_), stype0); + case SType::BOOL: return _rowfirstlast(std::move(columns), stype0); + case SType::INT8: return _rowfirstlast(std::move(columns), stype0); + case SType::INT16: return _rowfirstlast(std::move(columns), stype0); + case SType::INT32: return _rowfirstlast(std::move(columns), stype0); + case SType::INT64: return _rowfirstlast(std::move(columns), stype0); + case SType::FLOAT32: return _rowfirstlast(std::move(columns), stype0); + case SType::FLOAT64: return _rowfirstlast(std::move(columns), stype0); case SType::STR32: - case SType::STR64: return _rowfirstlast(std::move(columns_), stype0); + case SType::STR64: return _rowfirstlast(std::move(columns), stype0); default: { throw TypeError() << "Function `" << name() << "` doesn't support type `" << stype0 << "`"; } diff --git a/src/core/expr/fnary/rowmean.cc b/src/core/expr/fnary/rowmean.cc index 627348af08..1c5fe3d2aa 100644 --- a/src/core/expr/fnary/rowmean.cc +++ b/src/core/expr/fnary/rowmean.cc @@ -62,9 +62,12 @@ static inline Column _rowmean(colvec&& columns) { } -Column FExpr_RowMean::apply_function(colvec&& columns) const { +Column FExpr_RowMean::apply_function(colvec&& columns, + const size_t nrows, + const size_t) const +{ if (columns.empty()) { - return Const_ColumnImpl::make_na_column(1); + return Column(new ConstNa_ColumnImpl(nrows, SType::FLOAT64)); } SType res_stype = common_numeric_stype(columns); if (res_stype == SType::INT32 || res_stype == SType::INT64) { diff --git a/src/core/expr/fnary/rowminmax.cc b/src/core/expr/fnary/rowminmax.cc index 19f77b5e0c..c8ac874280 100644 --- a/src/core/expr/fnary/rowminmax.cc +++ b/src/core/expr/fnary/rowminmax.cc @@ -33,6 +33,12 @@ namespace dt { namespace expr { +template +FExpr_RowMinMax::FExpr_RowMinMax(ptrExpr&& args) + : FExpr_RowFn(std::move(args), ARG) +{} + + template std::string FExpr_RowMinMax::name() const { if (ARG) { @@ -90,9 +96,11 @@ static inline Column _rowminmax(colvec&& columns) { template -Column FExpr_RowMinMax::apply_function(colvec&& columns) const { +Column FExpr_RowMinMax::apply_function(colvec&& columns, + const size_t nrows, + const size_t) const { if (columns.empty()) { - return Const_ColumnImpl::make_na_column(1); + return Const_ColumnImpl::make_na_column(nrows); } SType res_stype = common_numeric_stype(columns); promote_columns(columns, res_stype); diff --git a/src/core/expr/fnary/rowsd.cc b/src/core/expr/fnary/rowsd.cc index c8decd1b61..b5b29424c2 100644 --- a/src/core/expr/fnary/rowsd.cc +++ b/src/core/expr/fnary/rowsd.cc @@ -66,9 +66,12 @@ static inline Column _rowsd(colvec&& columns) { } -Column FExpr_RowSd::apply_function(colvec&& columns) const { +Column FExpr_RowSd::apply_function(colvec&& columns, + const size_t nrows, + const size_t) const +{ if (columns.empty()) { - return Const_ColumnImpl::make_na_column(1); + return Column(new ConstNa_ColumnImpl(nrows, SType::FLOAT64)); } SType res_stype = common_numeric_stype(columns); if (res_stype == SType::INT32 || res_stype == SType::INT64) { diff --git a/src/core/expr/fnary/rowsum.cc b/src/core/expr/fnary/rowsum.cc index 679449fd3d..e73b11aa88 100644 --- a/src/core/expr/fnary/rowsum.cc +++ b/src/core/expr/fnary/rowsum.cc @@ -58,9 +58,11 @@ static inline Column _rowsum(colvec&& columns) { } -Column FExpr_RowSum::apply_function(colvec&& columns) const { +Column FExpr_RowSum::apply_function(colvec&& columns, + const size_t nrows, + const size_t) const { if (columns.empty()) { - return Const_ColumnImpl::make_int_column(1, 0, SType::INT32); + return Const_ColumnImpl::make_int_column(nrows, 0, SType::INT32); } SType res_stype = common_numeric_stype(columns); promote_columns(columns, res_stype); diff --git a/tests/ijby/test-rowwise.py b/tests/ijby/test-rowwise.py index 60dcfae737..e315c835b8 100644 --- a/tests/ijby/test-rowwise.py +++ b/tests/ijby/test-rowwise.py @@ -249,8 +249,6 @@ def test_rowfirstlast_incompatible_types(): - - #------------------------------------------------------------------------------- # rowmax(), rowmin() #------------------------------------------------------------------------------- @@ -267,6 +265,18 @@ def test_rowminmax_int8(): assert_equals(RES, dt.Frame([[4], [1]], stype=dt.int32)) +def test_rowminmax_void_column1(): + DT = dt.Frame([[None]] * 3) + RES = DT[:, [rowmax(f[:]), rowmin(f[:])]] + assert_equals(RES, dt.Frame([[None], [None]])) + + +def test_rowminmax_void_column2(): + DT = dt.Frame([[None], [None], [1.0], [None]]) + RES = DT[:, [rowmax(f[:]), rowmin(f[:])]] + assert_equals(RES, dt.Frame([[1.0], [1.0]])) + + def test_rowminmax_nas(): DT = dt.Frame([[None]] * 3, stype=dt.int64) RES = DT[:, [rowmax(f[:]), rowmin(f[:])]] @@ -309,6 +319,18 @@ def test_rowargminmax_int8(): assert_equals(RES, dt.Frame([[0], [2]], stype=dt.int64)) +def test_rowargminmax_void_column1(): + DT = dt.Frame([[None]] * 3) + RES = DT[:, [rowargmax(f[:]), rowargmin(f[:])]] + assert_equals(RES, dt.Frame([[None], [None]], stype=dt.int64)) + + +def test_rowargminmax_void_column2(): + DT = dt.Frame([[None], [None], [-100], [None], [1.0], [None]]) + RES = DT[:, [rowargmax(f[:]), rowargmin(f[:])]] + assert_equals(RES, dt.Frame([[4], [2]], stype=dt.int64)) + + def test_rowargminmax_nas(): DT = dt.Frame([[None]] * 3, stype=dt.int64) RES = DT[:, [rowargmax(f[:]), rowargmin(f[:])]] @@ -343,6 +365,18 @@ def test_rowmean_simple(): assert_equals(DT[:, rowmean(f[:])], dt.Frame(range(5), stype=dt.float64)) +def test_rowmean_void_column1(): + DT = dt.Frame([[None]] * 3) + RES = DT[:, rowmean(f[:])] + assert_equals(RES, dt.Frame([None], stype=dt.float64)) + + +def test_rowmean_void_column2(): + DT = dt.Frame([[None], [None], [4], [None], [6], [None]]) + RES = DT[:, rowmean(f[:])] + assert_equals(RES, dt.Frame([5], stype=dt.float64)) + + def test_rowmean_floats(): DT = dt.Frame([(1.5, 6.4, 0.0, None, 7.22), (2.0, -1.1, math.inf, 4.0, 3.2), @@ -361,7 +395,6 @@ def test_rowmean_wrong_types(): - #------------------------------------------------------------------------------- # rowsd() #------------------------------------------------------------------------------- @@ -372,6 +405,18 @@ def test_rowsd_single_column(): assert_equals(RES, dt.Frame([None]*5, type=float)) +def test_rowsd_void_column1(): + DT = dt.Frame([[None]] * 3) + RES = DT[:, rowsd(f[:])] + assert_equals(RES, dt.Frame([None], stype=dt.float64)) + + +def test_rowsd_void_column2(): + DT = dt.Frame([[None], [None], [6], [None], [6], [None]]) + RES = DT[:, rowsd(f[:])] + assert_equals(RES, dt.Frame([0], stype=dt.float64)) + + def test_rowsd_same_columns(): DT = dt.Frame([range(5)] * 10) RES = DT[:, rowsd(f[:])] @@ -397,11 +442,22 @@ def test_rowsd_wrong_types(): - #------------------------------------------------------------------------------- # rowsum() #------------------------------------------------------------------------------- +def test_rowsum_void_column1(): + DT = dt.Frame([[None]] * 3) + RES = DT[:, rowsum(f[:])] + assert_equals(RES, dt.Frame([0], stype=dt.int32)) + + +def test_rowsum_void_column2(): + DT = dt.Frame([[None], [4], [6], [None], [5], [None]]) + RES = DT[:, rowsum(f[:])] + assert_equals(RES, dt.Frame([15], stype=dt.int32)) + + def test_rowsum_bools(): DT = dt.Frame([[True, True, False, False, None, None], [True, False, True, False, True, None], From 33b35f7debc6b3dbe9f77371e2dc879c7bc7f4b8 Mon Sep 17 00:00:00 2001 From: Oleksiy Kononenko Date: Thu, 2 Jun 2022 16:41:18 -0700 Subject: [PATCH 2/6] Allow multiline function signatures --- docs/_ext/xfunction.py | 7 ++----- src/core/expr/fnary/rowall.cc | 3 ++- src/core/expr/fnary/rowany.cc | 3 ++- src/core/expr/fnary/rowfirstlast.cc | 3 ++- src/core/expr/fnary/rowmean.cc | 2 +- src/core/expr/fnary/rowminmax.cc | 4 +++- src/core/expr/fnary/rowsd.cc | 2 +- src/core/expr/fnary/rowsum.cc | 6 +++--- 8 files changed, 16 insertions(+), 14 deletions(-) diff --git a/docs/_ext/xfunction.py b/docs/_ext/xfunction.py index 712af84e5a..732be2b62b 100644 --- a/docs/_ext/xfunction.py +++ b/docs/_ext/xfunction.py @@ -1173,6 +1173,7 @@ def locate_cxx_function(name, kind, lines): r"\s*\(.*\)\s*" + r"(?:const\s*|noexcept\s*|override\s*)*" + r"\{\s*") + n_signature_lines = 5 # number of lines allowed for the function's signature expect_closing = None istart = None ifinish = None @@ -1183,13 +1184,9 @@ def locate_cxx_function(name, kind, lines): break elif name in line: istart = i - mm = re.match(rx_start, line) + mm = re.match(rx_start, "".join(lines[i:i+n_signature_lines])) if mm: expect_closing = mm.group(1) + "}" - else: - mm = re.match(rx_start, line + lines[i+1]) - if mm: - expect_closing = mm.group(1) + "}" if not istart: raise ValueError("Could not find %s `%s` in " % (kind, name)) if not expect_closing: diff --git a/src/core/expr/fnary/rowall.cc b/src/core/expr/fnary/rowall.cc index 435bfec581..7cece08163 100644 --- a/src/core/expr/fnary/rowall.cc +++ b/src/core/expr/fnary/rowall.cc @@ -51,7 +51,8 @@ static bool op_rowall(size_t i, int8_t* out, const colvec& columns) { Column FExpr_RowAll::apply_function(colvec&& columns, const size_t nrows, - const size_t ncols) const { + const size_t ncols) const +{ // No columns if (ncols == 0) { return Const_ColumnImpl::make_bool_column(1, true); diff --git a/src/core/expr/fnary/rowany.cc b/src/core/expr/fnary/rowany.cc index 0fa6cccffd..69f74f47d6 100644 --- a/src/core/expr/fnary/rowany.cc +++ b/src/core/expr/fnary/rowany.cc @@ -52,7 +52,8 @@ static bool op_rowany(size_t i, int8_t* out, const colvec& columns) { Column FExpr_RowAny::apply_function(colvec&& columns, const size_t nrows, - const size_t ncols) const { + const size_t ncols) const +{ // No non-void columns if (columns.empty()) { // `ncols == 0` tests that there is no void columns diff --git a/src/core/expr/fnary/rowfirstlast.cc b/src/core/expr/fnary/rowfirstlast.cc index b532a6b32f..3a1c9f3b6a 100644 --- a/src/core/expr/fnary/rowfirstlast.cc +++ b/src/core/expr/fnary/rowfirstlast.cc @@ -59,7 +59,8 @@ static inline Column _rowfirstlast(colvec&& columns, SType outtype) { template Column FExpr_RowFirstLast::apply_function(colvec&& columns, const size_t nrows, - const size_t) const { + const size_t) const +{ if (columns.empty()) { return Const_ColumnImpl::make_na_column(nrows); } diff --git a/src/core/expr/fnary/rowmean.cc b/src/core/expr/fnary/rowmean.cc index 1c5fe3d2aa..687d378a16 100644 --- a/src/core/expr/fnary/rowmean.cc +++ b/src/core/expr/fnary/rowmean.cc @@ -84,6 +84,7 @@ Column FExpr_RowMean::apply_function(colvec&& columns, } } + DECLARE_PYFN(&py_rowfn) ->name("rowmean") ->docs(doc_dt_rowmean) @@ -92,5 +93,4 @@ DECLARE_PYFN(&py_rowfn) - }} // namespace dt::expr diff --git a/src/core/expr/fnary/rowminmax.cc b/src/core/expr/fnary/rowminmax.cc index c8ac874280..fbba48ae7f 100644 --- a/src/core/expr/fnary/rowminmax.cc +++ b/src/core/expr/fnary/rowminmax.cc @@ -98,7 +98,8 @@ static inline Column _rowminmax(colvec&& columns) { template Column FExpr_RowMinMax::apply_function(colvec&& columns, const size_t nrows, - const size_t) const { + const size_t) const +{ if (columns.empty()) { return Const_ColumnImpl::make_na_column(nrows); } @@ -116,6 +117,7 @@ Column FExpr_RowMinMax::apply_function(colvec&& columns, } } + template class FExpr_RowMinMax; template class FExpr_RowMinMax; template class FExpr_RowMinMax; diff --git a/src/core/expr/fnary/rowsd.cc b/src/core/expr/fnary/rowsd.cc index b5b29424c2..949bc5530b 100644 --- a/src/core/expr/fnary/rowsd.cc +++ b/src/core/expr/fnary/rowsd.cc @@ -88,6 +88,7 @@ Column FExpr_RowSd::apply_function(colvec&& columns, } } + DECLARE_PYFN(&py_rowfn) ->name("rowsd") ->docs(doc_dt_rowsd) @@ -96,5 +97,4 @@ DECLARE_PYFN(&py_rowfn) - }} // namespace dt::expr diff --git a/src/core/expr/fnary/rowsum.cc b/src/core/expr/fnary/rowsum.cc index e73b11aa88..316c5c3475 100644 --- a/src/core/expr/fnary/rowsum.cc +++ b/src/core/expr/fnary/rowsum.cc @@ -29,7 +29,6 @@ namespace dt { namespace expr { - std::string FExpr_RowSum::name() const { return "rowsum"; } @@ -60,7 +59,8 @@ static inline Column _rowsum(colvec&& columns) { Column FExpr_RowSum::apply_function(colvec&& columns, const size_t nrows, - const size_t) const { + const size_t) const +{ if (columns.empty()) { return Const_ColumnImpl::make_int_column(nrows, 0, SType::INT32); } @@ -78,6 +78,7 @@ Column FExpr_RowSum::apply_function(colvec&& columns, } } + DECLARE_PYFN(&py_rowfn) ->name("rowsum") ->docs(doc_dt_rowsum) @@ -86,5 +87,4 @@ DECLARE_PYFN(&py_rowfn) - }} // namespace dt::expr From 3ae743b7a0a5a91d3b24b7ba571a6b878b0ad094 Mon Sep 17 00:00:00 2001 From: Oleksiy Kononenko Date: Thu, 2 Jun 2022 16:48:13 -0700 Subject: [PATCH 3/6] Cosmetics --- docs/_ext/xfunction.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/_ext/xfunction.py b/docs/_ext/xfunction.py index 732be2b62b..210e3cdab6 100644 --- a/docs/_ext/xfunction.py +++ b/docs/_ext/xfunction.py @@ -1184,7 +1184,8 @@ def locate_cxx_function(name, kind, lines): break elif name in line: istart = i - mm = re.match(rx_start, "".join(lines[i:i+n_signature_lines])) + txt = line.join(lines[i+1:i+n_signature_lines] + mm = re.match(rx_start, txt) if mm: expect_closing = mm.group(1) + "}" if not istart: From 05d67aee8e5f65d9957d69bba3f96f6db5570571 Mon Sep 17 00:00:00 2001 From: Oleksiy Kononenko Date: Thu, 2 Jun 2022 16:56:29 -0700 Subject: [PATCH 4/6] Fix typo --- docs/_ext/xfunction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_ext/xfunction.py b/docs/_ext/xfunction.py index 210e3cdab6..eaa88fb210 100644 --- a/docs/_ext/xfunction.py +++ b/docs/_ext/xfunction.py @@ -1184,7 +1184,7 @@ def locate_cxx_function(name, kind, lines): break elif name in line: istart = i - txt = line.join(lines[i+1:i+n_signature_lines] + txt = "".join(lines[i:i+n_signature_lines]) mm = re.match(rx_start, txt) if mm: expect_closing = mm.group(1) + "}" From 8544cb064ac0f9e6bcf5fe16760171c8b237c3bc Mon Sep 17 00:00:00 2001 From: Oleksiy Kononenko Date: Thu, 2 Jun 2022 17:28:50 -0700 Subject: [PATCH 5/6] Revert some changes to make it faster --- docs/_ext/xfunction.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/_ext/xfunction.py b/docs/_ext/xfunction.py index eaa88fb210..4405f7a69a 100644 --- a/docs/_ext/xfunction.py +++ b/docs/_ext/xfunction.py @@ -1184,10 +1184,15 @@ def locate_cxx_function(name, kind, lines): break elif name in line: istart = i - txt = "".join(lines[i:i+n_signature_lines]) - mm = re.match(rx_start, txt) + mm = re.match(rx_start, line) if mm: expect_closing = mm.group(1) + "}" + else: + txt = "".join(lines[i:i+n_signature_lines]) + mm = re.match(rx_start, txt) + if mm: + expect_closing = mm.group(1) + "}" + if not istart: raise ValueError("Could not find %s `%s` in " % (kind, name)) if not expect_closing: From 9beb9e7df3545f1f7fc73104ab69e203af772ab6 Mon Sep 17 00:00:00 2001 From: Oleksiy Kononenko Date: Thu, 2 Jun 2022 22:34:35 -0700 Subject: [PATCH 6/6] Cosmetics and a changelog --- docs/_ext/xfunction.py | 6 +++--- docs/releases/v1.1.0.rst | 3 +++ src/core/expr/fnary/fnary.cc | 3 ++- src/core/expr/fnary/rowany.cc | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/_ext/xfunction.py b/docs/_ext/xfunction.py index 4405f7a69a..dd600aac94 100644 --- a/docs/_ext/xfunction.py +++ b/docs/_ext/xfunction.py @@ -1173,7 +1173,7 @@ def locate_cxx_function(name, kind, lines): r"\s*\(.*\)\s*" + r"(?:const\s*|noexcept\s*|override\s*)*" + r"\{\s*") - n_signature_lines = 5 # number of lines allowed for the function's signature + n_signature_lines = 5 # number of lines allowed for the function signature expect_closing = None istart = None ifinish = None @@ -1188,8 +1188,8 @@ def locate_cxx_function(name, kind, lines): if mm: expect_closing = mm.group(1) + "}" else: - txt = "".join(lines[i:i+n_signature_lines]) - mm = re.match(rx_start, txt) + src = "".join(lines[i:i+n_signature_lines]) + mm = re.match(rx_start, src) if mm: expect_closing = mm.group(1) + "}" diff --git a/docs/releases/v1.1.0.rst b/docs/releases/v1.1.0.rst index 22ee9acef7..2eaea00a0a 100644 --- a/docs/releases/v1.1.0.rst +++ b/docs/releases/v1.1.0.rst @@ -97,6 +97,9 @@ -[fix] Reducer functions :func:`dt.prod()` and :func:`dt.sum()` can now be applied to :attr:`void ` columns. [#3281] [#3282] + -[fix] All the row-wise functions now support :attr:`void ` + columns. [#3284] + fread ----- diff --git a/src/core/expr/fnary/fnary.cc b/src/core/expr/fnary/fnary.cc index 663afbb396..1174b837df 100644 --- a/src/core/expr/fnary/fnary.cc +++ b/src/core/expr/fnary/fnary.cc @@ -51,8 +51,9 @@ Workframe FExpr_RowFn::evaluate_n(EvalContext& ctx) const { size_t ncols = inputs.ncols(); size_t nrows = 1; columns.reserve(ncols); - for (size_t i = 0; i < inputs.ncols(); ++i) { + for (size_t i = 0; i < ncols; ++i) { Column col = inputs.retrieve_column(i); + xassert(i == 0 || nrows == col.nrows()); nrows = col.nrows(); if (process_void_cols_ || !col.type().is_void()) { columns.emplace_back(col); diff --git a/src/core/expr/fnary/rowany.cc b/src/core/expr/fnary/rowany.cc index 69f74f47d6..5401b2497f 100644 --- a/src/core/expr/fnary/rowany.cc +++ b/src/core/expr/fnary/rowany.cc @@ -54,9 +54,9 @@ Column FExpr_RowAny::apply_function(colvec&& columns, const size_t nrows, const size_t ncols) const { - // No non-void columns + // No columns or all the columns are void if (columns.empty()) { - // `ncols == 0` tests that there is no void columns + // `ncols == 0` tests that the original input frame had no columns return Const_ColumnImpl::make_bool_column(nrows, ncols == 0); } for (size_t i = 0; i < columns.size(); ++i) {