Correct treatment of ncells=NA for non-character columns

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: tiledb
 Type: Package
-Version: 0.24.0.7
+Version: 0.24.0.8
 Title: Modern Database Engine for Multi-Modal Data via Sparse and Dense Multidimensional Arrays
 Authors@R: c(person("TileDB, Inc.", role = c("aut", "cph")),
  person("Dirk", "Eddelbuettel", email = "dirk@tiledb.com", role = "cre"))

NEWS.md

@@ -14,6 +14,8 @@
 
 * The nighly valgrind check now installs to require `nanoarrow` package (#664)
 
+* Variable cell numbers can now set consistently for all attribute types (#670)
+
 ## Build and Test Systems
 
 * The nighly valgrind run was updated to include release 2.21 (#669)

R/Attribute.R

@@ -63,10 +63,13 @@ tiledb_attr <- function(name,
                         ctx = tiledb_get_context()
                         ) {
     if (missing(name)) name <- ""
-    stopifnot(`The 'type' argument for is mandatory` = !missing(type),
-              `The 'ctx' argument must be a tiledb_ctx` = is(ctx, "tiledb_ctx"),
-              `The 'name' argument must be a scalar string` = is.scalar(name, "character"),
-              `The 'filter_list' argument must be a tiledb_filter_list instance` = is(filter_list, "tiledb_filter_list"))
+    if (is.na(ncells)) ncells <- NA_integer_ 		# the specific NA for ints (as basic NA is bool)
+    stopifnot("The 'name' argument must be a scalar string" = is.scalar(name, "character"),
+              "The 'type' argument is mandatory" = !missing(type),
+              "The 'ncells' argument must be numeric or NA" = is.numeric(ncells) || is.na(ncells),
+              "The 'filter_list' argument must be a tiledb_filter_list instance" =
+                  is(filter_list, "tiledb_filter_list"),
+              "The 'ctx' argument must be a tiledb_ctx" = is(ctx, "tiledb_ctx"))
     ptr <- libtiledb_attribute(ctx@ptr, name, type, filter_list@ptr, ncells, nullable)
     attr <- new("tiledb_attr", ptr = ptr)
     if (!is.null(enumeration))

src/libtiledb.cpp

@@ -1394,11 +1394,12 @@ XPtr<tiledb::Attribute> libtiledb_attribute(XPtr<tiledb::Context> ctx,
                                             int ncells,
                                             bool nullable) {
     check_xptr_tag<tiledb::Context>(ctx);
+    spdl::debug(tfm::format("[libtiledb_attribute] Attr name %s type %s ncells %d nullable %s",
+                            name, type, ncells, nullable ? "true" : "false"));
     tiledb_datatype_t attr_dtype = _string_to_tiledb_datatype(type);
     if (ncells < 1 && ncells != R_NaInt) {
         Rcpp::stop("ncells must be >= 1 (or NA for variable cells)");
     }
-
     // placeholder, overwritten in all branches below
     XPtr<tiledb::Attribute> attr = XPtr<tiledb::Attribute>(static_cast<tiledb::Attribute*>(nullptr));
 
@@ -1432,11 +1433,6 @@ XPtr<tiledb::Attribute> libtiledb_attribute(XPtr<tiledb::Context> ctx,
                attr_dtype == TILEDB_STRING_ASCII ||
                attr_dtype == TILEDB_STRING_UTF8) {
         attr = make_xptr<tiledb::Attribute>(new tiledb::Attribute(*ctx.get(), name, attr_dtype));
-        uint64_t num = static_cast<uint64_t>(ncells);
-        if (ncells == R_NaInt) {
-            num = TILEDB_VAR_NUM;           // R's NA is different from TileDB's NA
-        }
-        attr->set_cell_val_num(num);
 #if TILEDB_VERSION >= TileDB_Version(2,10,0)
     } else if (attr_dtype == TILEDB_BOOL) {
         attr = make_xptr<tiledb::Attribute>(new tiledb::Attribute(*ctx.get(), name, attr_dtype));
@@ -1450,6 +1446,9 @@ XPtr<tiledb::Attribute> libtiledb_attribute(XPtr<tiledb::Context> ctx,
                    "and character (CHAR,ASCII,UTF8) attributes are supported "
                    "-- seeing %s which is not", type.c_str());
     }
+    // R's NA is different from TileDB's NA so test for NA_integer_, else cast
+    uint64_t num = (ncells == R_NaInt) ? TILEDB_VAR_NUM : static_cast<uint64_t>(ncells);
+    attr->set_cell_val_num(num);
     attr->set_filter_list(*fltrlst);
     attr->set_nullable(nullable);
     return attr;