Add R list-column support for FITS vector columns by Copilot · Pull Request #29 · asgr/Rfits

Copilot · 2026-06-15T06:57:21Z

FITS binary tables support vector columns (multiple elements per cell, e.g. TFORM=3D). This adds read/write support mapping them to R list-columns (I(list(...))), with efficient bulk I/O at the C++ level.

Changes

C++ (`src/Rfits.cpp`)

Read: When repeat > 1 (vector column), reads all nrow × repeat elements in a single CFITSIO call, then splits into an R list of vectors via memcpy — no per-element R allocation overhead. TBIT/TLOGICAL vector columns return a list of LogicalVector.
Write: New Cfits_write_col_vector flattens a list of vectors into a contiguous buffer and writes in one call. Supports double, integer, integer64, and logical element types. Guards against zero-length vectors and raises an explicit error for unsupported typecodes.

R (`R/Rfits_table.R`)

Write path: Detects list-columns, validates uniform vector lengths (regardless of whether tforms is auto or caller-supplied), generates correct TFORM (e.g. "3D", "4J", "5K", "2L")
Read path: Wraps returned list-columns with I() for proper data.frame/data.table semantics

Tests

Round-trip test with mixed scalar + vector columns (double, integer, and logical)
Round-trip test for integer64 list-columns
Error test for inconsistent vector lengths across rows

Usage

tb <- data.frame(
  id = 1:3,
  spectrum = I(list(rnorm(100), rnorm(100), rnorm(100))),
  flags    = I(list(1:5, 1:5, 1:5)),
  mask     = I(list(c(TRUE, FALSE), c(FALSE, FALSE), c(TRUE, TRUE)))
)

Rfits_write_table(tb, "vectors.fits")
tb2 <- Rfits_read_table("vectors.fits")
# tb2$spectrum is a list-column of length-100 numeric vectors
# tb2$mask is a list-column of logical vectors

- Cfits_read_col now returns an R list when repeat > 1 (vector column) - Added Cfits_write_col_vector for writing list-columns efficiently - Rfits_write_table detects list-columns and generates proper TFORMs - Rfits_read_table wraps vector columns with I() for data.frame/data.table - Added unit test (ex 49) for vector column round-trip

Copilot

Pull request overview

Adds read/write support for FITS binary table vector columns (TFORM repeat > 1) by mapping them to R list-columns (I(list(...))), with bulk I/O performed in C++ to avoid per-element overhead.

Changes:

src/Rfits.cpp: Adds vector-column read path in Cfits_read_col() and a new bulk writer Cfits_write_col_vector().
R/Rfits_table.R: Detects list-columns on write (generates appropriate TFORM/typecodes) and wraps list-columns with I() on read for data.frame/data.table semantics.
Tests/docs/version: Adds a round-trip test for vector columns, updates Rd examples, and bumps package version/date.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`tests/testthat/test_Rfits.R`	Adds a vector-column round-trip test (list-columns).
`src/Rfits.cpp`	Implements bulk read for repeat>1 columns and adds a bulk list-column writer.
`src/RcppExports.cpp`	Registers the new exported C++ symbol for `.Call`.
`R/Rfits_table.R`	Adds list-column detection/TFORM generation and routes vector writes to new C++ API.
`R/RcppExports.R`	Adds the R wrapper for `Cfits_write_col_vector()`.
`man/Rfits_table.Rd`	Documents vector-column usage and adds examples.
`DESCRIPTION`	Version/date bump.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-06-15T07:15:45Z

+    else if ( typecode == TINT || typecode == TINT32BIT || typecode == TLONG ||
+              typecode == TSHORT || typecode == TUSHORT || typecode == TBYTE ||
+              typecode == TUINT || typecode == TBIT || typecode == TLOGICAL ) {
+      // Read all elements as int
+      int nullval = -999;
+      std::vector<int> flat(total);
+      fits_invoke(read_col, fptr, TINT, colref, startrow, 1, total, &nullval, flat.data(), &anynull);
+
+      Rcpp::List out(nrow);
+      for (long i = 0; i < nrow; i++) {
+        Rcpp::IntegerVector v(repeat);
+        std::memcpy(&v[0], &flat[i * repeat], repeat * sizeof(int));
+        out[i] = v;
+      }
+      return out;
+    }


Fixed in commit. TBIT and TLOGICAL are now handled in a separate branch in the vector-column path that reads via Rbyte and converts to Rcpp::LogicalVector, matching the scalar behaviour.

Copilot · 2026-06-15T07:15:51Z

+  if (typecode == TDOUBLE || typecode == TFLOAT) {
+    // Flatten list of numeric vectors into contiguous buffer
+    std::vector<double> flat(nrow * vec_len);
+    for (long i = 0; i < nrow; i++) {
+      Rcpp::NumericVector v = Rcpp::as<Rcpp::NumericVector>(data[i]);
+      std::memcpy(&flat[i * vec_len], &v[0], vec_len * sizeof(double));
+    }
+    fits_invoke(write_col, fptr, TDOUBLE, colref, 1, 1, nrow * vec_len, flat.data());
+  } else if (typecode == TINT) {
+    std::vector<int> flat(nrow * vec_len);
+    for (long i = 0; i < nrow; i++) {
+      Rcpp::IntegerVector v = Rcpp::as<Rcpp::IntegerVector>(data[i]);
+      std::memcpy(&flat[i * vec_len], &v[0], vec_len * sizeof(int));
+    }
+    fits_invoke(write_col, fptr, TINT, colref, 1, 1, nrow * vec_len, flat.data());
+  } else if (typecode == TLONGLONG) {
+    std::vector<int64_t> flat(nrow * vec_len);
+    for (long i = 0; i < nrow; i++) {
+      Rcpp::NumericVector v = Rcpp::as<Rcpp::NumericVector>(data[i]);
+      std::memcpy(&flat[i * vec_len], &v[0], vec_len * sizeof(int64_t));
+    }
+    fits_invoke(write_col, fptr, TLONGLONG, colref, 1, 1, nrow * vec_len, flat.data());
+  }


Fixed in the same commit. Cfits_write_col_vector now returns early with Rcpp::stop when vec_len <= 0, checks each row's length against vec_len before memcpy, and throws an explicit error for unrecognised typecodes.

Copilot · 2026-06-15T07:15:56Z

+    if(check.list[i]){
+      # Write vector (list) column
+      vec_len = length(table[[i]][[1]])
+      Cfits_write_col_vector(filename=filename, data=table[[i]], nrow=nrow, vec_len=vec_len, colref=i, ext=ext, typecode=typecode[i])
+    }else{


Fixed. The write loop now always calls sapply(table[[i]], length) and validates uniform length before forwarding to Cfits_write_col_vector, regardless of whether tforms was auto-detected or caller-supplied.

Copilot · 2026-06-15T07:16:02Z

+    if(length(grep('1B|1K|1J|1D|1E|1I|A|[0-9]+[JKDEI]',tforms)) != ncol){
+      stop(cat('Unrecognised column data type in column', paste(which(!1:ncol %in% grep('1B|1K|1J|1D|1E|1I|A|[0-9]+[JKDEI]',tforms))),sep='\n'))
    }


Fixed. Replaced stop(cat(...)) with an explicit invalid index built via which(!(1:ncol %in% grep(...))) and a single stop(...) call, fixing both the stdout-print issue and the !1:ncol precedence bug.

Copilot · 2026-06-15T07:16:14Z

+#ex 49 write and read vector (list) columns
+file_vec_table = tempfile()
+tb_vec = data.frame(
+  id = 1:3,
+  vals_dbl = I(list(c(1.1, 2.2, 3.3), c(4.4, 5.5, 6.6), c(7.7, 8.8, 9.9))),
+  vals_int = I(list(1:4, 5:8, 9:12))
+)
+Rfits_write_table(tb_vec, file_vec_table)
+tb_vec_read = Rfits_read_table(file_vec_table)
+expect_identical(tb_vec$id, tb_vec_read$id)
+expect_equal(tb_vec$vals_dbl, as.list(tb_vec_read$vals_dbl))
+expect_identical(tb_vec$vals_int, as.list(tb_vec_read$vals_int))


Added two new test blocks: ex 50 round-trips a bit64::integer64 list column and verifies the values, and ex 51 asserts that a table with inconsistent vector lengths throws an error matching "inconsistent vector lengths".

…tforms validation, typo, and integer64 tests

asgr · 2026-06-15T07:25:25Z

@copilot currently vector columns with logical data types are not being written correctly:

file_vec_table = tempfile()
tb_vec = data.frame(

```
id = 1:3,
```

vals_dbl = I(list(c(1.1, 2.2, 3.3), c(4.4, 5.5, 6.6), c(7.7, 8.8, 9.9))),

```
vals_int = I(list(1:4, 5:8, 9:12)),
```

vals_lgc = I(list(c(T,T), c(F,T), c(F,F)))

)

Rfits_write_table(tb_vec, file_vec_table)
tb_vec_read = Rfits_read_table(file_vec_table)
expect_identical(tb_vec$vals_lgc, as.list(tb_vec_read$vals_lgc))
Error: tb_vec$vals_lgc not identical to as.list(tb_vec_read$vals_lgc).
Component 1: Modes: logical, numeric
Component 1: target is logical, current is numeric
Component 2: Modes: logical, numeric
Component 2: target is logical, current is numeric
Component 3: Modes: logical, numeric
Component 3: target is logical, current is numeric

Copilot · 2026-06-15T07:30:43Z

@copilot currently vector columns with logical data types are not being written correctly:

file_vec_table = tempfile()
...

Fixed. The write path now detects is.logical(first_elem) for list-columns and assigns TFORM "NL" / typecode 14 (TLOGICAL). A corresponding branch was added to Cfits_write_col_vector in C++ to flatten LogicalVector rows into an int buffer and write via TLOGICAL. The validation regex was also updated to accept L tforms. A round-trip test for vals_lgc is included in the test suite.

…lums, since it turns out that wasn't working properly)

…st, but much more support than there was).

Copilot

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 9 comments.

+  }else if (typecode == TLOGICAL){
+
+    std::vector<char> l_data(nrow);
+    int *r_data = INTEGER(data);
+
+    for (long i = 0; i < nrow; i++) {
+      if (r_data[i] == NA_LOGICAL) {
+        l_data[i] = 0;   // or handle via nullmask if needed
+      } else {
+        l_data[i] = r_data[i] == 1 ? 1 : 0;
+      }
+    }
+
+    fits_invoke(write_col, fptr, TLOGICAL, colref, 1, 1, nrow, l_data.data());
+  }else if (typecode == TBYTE || typecode == TINT){


+  }else if (typecode == TBYTE || typecode == TINT){
    fits_invoke(write_col, fptr, typecode, colref, 1, 1, nrow, INTEGER(data));
-  }else if(typecode == TLONGLONG){
-    fits_invoke(write_col, fptr, typecode, colref, 1, 1, nrow, REAL(data));
-  }else if(typecode == TDOUBLE){
+  }else if(typecode == TLONGLONG || typecode == TDOUBLE || typecode == TFLOAT){
    fits_invoke(write_col, fptr, typecode, colref, 1, 1, nrow, REAL(data));
  }


+    else if ( typecode == TBIT || typecode == TLOGICAL ) {
+      // Read as bytes (0/non-zero), convert to LogicalVector per row
+      int nullval = 0;
+      std::vector<Rbyte> flat(total);
+      fits_invoke(read_col, fptr, typecode, colref, startrow, 1, total, &nullval, flat.data(), &anynull);


+  } else if (typecode == TDOUBLE || typecode == TFLOAT) {
+    // Flatten list of numeric vectors into contiguous buffer
+    std::vector<double> flat(nrow * vec_len);
+    for (long i = 0; i < nrow; i++) {
+      Rcpp::NumericVector v = Rcpp::as<Rcpp::NumericVector>(data[i]);
+      if ((long)v.size() != vec_len) {
+        Rcpp::stop("Vector column row %ld has length %ld, expected %ld", i + 1, (long)v.size(), vec_len);
+      }
+      std::memcpy(&flat[i * vec_len], &v[0], vec_len * sizeof(double));
+    }
+    fits_invoke(write_col, fptr, typecode, colref, 1, 1, nrow * vec_len, flat.data());
+  } else if (typecode == TINT || typecode == TBYTE) {
+    std::vector<int> flat(nrow * vec_len);
+    for (long i = 0; i < nrow; i++) {
+      Rcpp::IntegerVector v = Rcpp::as<Rcpp::IntegerVector>(data[i]);
+      if ((long)v.size() != vec_len) {
+        Rcpp::stop("Vector column row %ld has length %ld, expected %ld", i + 1, (long)v.size(), vec_len);
+      }
+      std::memcpy(&flat[i * vec_len], &v[0], vec_len * sizeof(int));
+    }
+    fits_invoke(write_col, fptr, typecode, colref, 1, 1, nrow * vec_len, flat.data());
+  } else if (typecode == TLONGLONG) {


+    if(is.character(output[[count]])){
+      if(all(output[[count]] %in% c('T', 'F', ' '))){
+        is_TRUE = (output[[count]] == 'T')
+        is_NA = (output[[count]] == ' ')
+        output[[count]] = is_TRUE
+        output[[count]][is_NA] = NA
+      }


+          lens = sapply(table[[i]], length)
+          if(length(unique(lens)) != 1){
+            stop("Vector column '", ttypes[i], "' has inconsistent vector lengths across rows")
+          }
+          vec_len = lens[1]
+          first_elem = table[[i]][[1]]
+          if(is.logical(first_elem)){
+            tforms[i] = paste0(vec_len, "L")
+          }else if(is.integer(first_elem)){
+            tforms[i] = paste0(vec_len, "J")
+          }else if(is.integer64(first_elem)){
+            tforms[i] = paste0(vec_len, "K")
+          }else{
+            tforms[i] = paste0(vec_len, "D")
+          }


+      first_elem = table[[i]][[1]]
+      if(is.logical(first_elem)){
+        typecode[i] = 14 # TLOGICAL
+      }else if(is.integer(first_elem)){
+        typecode[i] = 31 # TINT
+      }else if(is.integer64(first_elem)){
+        typecode[i] = 81 # TLONGLONG
+      }else if(is.double(first_elem)){
+        typecode[i] = 82 # TDOUBLE
+      }else if(is.character(first_elem)){
+        typecode[i] = 16 # CHAR
+      }
+    }


+      if(table_type == 1){
+        if(is.logical(table[[i]])){
+          sel_TRUE = which(table[[i]])
+          sel_FALSE = which(!table[[i]])
+          table[[i]] = rep(' ', length(table[[i]]))
+          table[[i]][sel_TRUE] = 'T'
+          table[[i]][sel_FALSE] = 'F'
+        }else{
+          if(anyNA(table[[i]])){
+            table[[i]][is.na(table[[i]])] = NA_replace
+          }
+          if(anyNaN(table[[i]])){
+            table[[i]][is.nan(table[[i]])] = NaN_replace
+          }
+          if(anyInfinite(table[[i]])){
+            table[[i]][is.infinite(table[[i]])] = Inf_replace
+          }
+        }
+      }


asgr · 2026-06-16T12:54:41Z

@copilot apply the most critical fixes of bugs based on the recent copilot code review. Highly critical ones are related to proper NA support via null masking, and type mismatches that could cause nonsense outputs. Other ones can be implemented if code changes are fairly minor.

- Vector logical read: use fits_read_colnull with null mask to restore NA_LOGICAL - Scalar logical write (Cfits_write_col): use LOGICAL() macro (not INTEGER()), write NAs via fits_write_colnull with a dedicated null marker (value 2) to avoid misidentifying TRUE/FALSE as null - Vector logical write (Cfits_write_col_vector): replace nullmask-array approach (which was incorrectly passing nullmask.data() as the nulval pointer) with the same null marker approach - Cfits_write_col TBYTE: pass TINT datatype so CFITSIO interprets the R int buffer correctly instead of reading it as unsigned char - Cfits_write_col TFLOAT: pass TDOUBLE datatype so CFITSIO reads the R double buffer correctly instead of interpreting it as a float pointer - Cfits_write_col_vector TFLOAT/TDOUBLE: always pass TDOUBLE to fits_write_col - Cfits_write_col_vector TINT/TBYTE: always pass TINT to fits_write_col - R tform auto-generation: add zero-length vector check; make unsupported element types an explicit error (not silent fallthrough to double) - R typecodes: remove character list-column case (unsupported in C++); explicit error for unsupported element types - NA/NaN/Inf replacement: apply to all numeric columns regardless of table_type, not just ASCII tables - Restore full binary vs ASCII table comparison in test (was narrowed to 30 cols)

Copilot · 2026-06-16T13:05:37Z

@copilot apply the most critical fixes of bugs based on the recent copilot code review. Highly critical ones are related to proper NA support via null masking, and type mismatches that could cause non...

Applied all critical fixes in commit $(git rev-parse --short HEAD 2>/dev/null || echo 'latest'):

NA null-masking:

Vector logical write: the previous nullmask.data() was being passed as CFITSIO's nulval (a single-value pointer), so CFITSIO was misidentifying real TRUE/FALSE values as FITS nulls. Fixed to use a dedicated null marker value (2, which is neither FALSE/0 nor TRUE/1) in both Cfits_write_col_vector and the scalar Cfits_write_col path.
Scalar logical write also switched from INTEGER(data) (undefined behaviour on LGLSXP) to LOGICAL(data), and uses fits_write_colnull when NAs are present.
Vector logical read now uses fits_read_colnull with a null mask, mapping null cells back to NA_LOGICAL.

Type mismatches:

Cfits_write_col TBYTE: now passes TINT as the datatype so CFITSIO reads the R int* buffer correctly.
Cfits_write_col TFLOAT: now passes TDOUBLE so CFITSIO reads the R double* buffer correctly.
Cfits_write_col_vector TFLOAT/TDOUBLE: always passes TDOUBLE; TINT/TBYTE: always passes TINT.

Additional minor fixes:

NA/NaN/Inf replacement now applied to binary table numeric columns too (not just ASCII).
Zero-length vector column and unsupported element type errors raised early in R before reaching C++.
Removed unsupported character list-column typecode path.
Restored full binary vs ASCII table comparison in test.

…. Need to check that, but a problem for anohter day I think (wasn't like we supported it properly before either).

Copilot AI and others added 3 commits June 15, 2026 04:33

fix: add validation for consistent vector lengths in list-columns

2c4ea3b

Added some examples and doc examplanation for new vector column support.

9c60baf

Copilot AI assigned Copilot and asgr Jun 15, 2026

Copilot created this pull request from a session on behalf of asgr June 15, 2026 06:57 View session

asgr requested a review from Copilot June 15, 2026 06:57

Copilot started reviewing on behalf of asgr June 15, 2026 06:57 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

Copilot started work on behalf of asgr June 15, 2026 07:09 View session

Fix review comments: TBIT/TLOGICAL vectors, write_col_vector guards, …

e341c22

…tforms validation, typo, and integer64 tests

Copilot finished work on behalf of asgr June 15, 2026 07:16

Copilot AI requested a review from asgr June 15, 2026 07:16

Copilot started work on behalf of asgr June 15, 2026 07:26 View session

Add logical vector column write/read support and test

3177126

Copilot finished work on behalf of asgr June 15, 2026 07:31

asgr added 3 commits June 15, 2026 17:03

Updates to support logical flags in vector columns (also in normal co…

488f8d2

…lums, since it turns out that wasn't working properly)

And for keyvalues

a8a9cd4

Handles logicals pretty well now (perhaps some corner cases still exi…

c326825

…st, but much more support than there was).

asgr requested a review from Copilot June 16, 2026 08:18

Copilot started reviewing on behalf of asgr June 16, 2026 08:19 View session

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Copilot started work on behalf of asgr June 16, 2026 12:55 View session

Copilot finished work on behalf of asgr June 16, 2026 13:06

NA are working on write out (checked on TopCat) but not fully on read…

13dc15a

…. Need to check that, but a problem for anohter day I think (wasn't like we supported it properly before either).

asgr marked this pull request as ready for review June 16, 2026 15:21

asgr merged commit f2c9e50 into master Jun 16, 2026
8 checks passed

asgr deleted the copilot/add-list-column-support branch June 16, 2026 15:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add R list-column support for FITS vector columns#29

Add R list-column support for FITS vector columns#29
asgr merged 10 commits into
masterfrom
copilot/add-list-column-support

Copilot AI commented Jun 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 15, 2026

Uh oh!

Copilot AI Jun 15, 2026

Uh oh!

Copilot AI Jun 15, 2026

Uh oh!

Copilot AI Jun 15, 2026

Uh oh!

Uh oh!

Copilot AI Jun 15, 2026

Uh oh!

asgr commented Jun 15, 2026

Uh oh!

Copilot AI commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

asgr commented Jun 16, 2026

Uh oh!

Copilot AI commented Jun 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

C++ (src/Rfits.cpp)

R (R/Rfits_table.R)

Tests

Usage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

asgr commented Jun 15, 2026

Uh oh!

Copilot AI commented Jun 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

asgr commented Jun 16, 2026

Uh oh!

Copilot AI commented Jun 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Jun 15, 2026 •

edited

Loading

C++ (`src/Rfits.cpp`)

R (`R/Rfits_table.R`)