From 9a636773b7bbcd6a5655bec2bc7cfa3e4853d131 Mon Sep 17 00:00:00 2001 From: David Anthoff Date: Sun, 30 Aug 2026 23:00:25 -0700 Subject: [PATCH 1/4] Pin ReadStat_jll to 1.1.9 and raise Julia floor to 1.10 The old bounds did not actually deliver the latest binary. ReadStat_jll 1.1.8 and newer require julia 1.6, but our julia compat floor was 1.3 and our ReadStat_jll bound was "1.1.1" (i.e. [1.1.1, 2.0)). On julia 1.3-1.5 the resolver therefore fell back to ReadStat_jll 1.1.5, shipping the December 2020 readstat build rather than 1.1.9. Pin ReadStat_jll to "1.1.9" and raise the julia floor to 1.10, matching Query.jl and ReadStatTables.jl. Raising the floor is breaking for old-julia users, so bump the version to 1.2.0-DEV. Co-Authored-By: Claude Opus 5 --- Project.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Project.toml b/Project.toml index 98855cc..c833804 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "ReadStat" uuid = "d71aba96-b539-5138-91ee-935c3ee1374c" -version = "1.1.2-DEV" +version = "1.2.0-DEV" [deps] Dates = "ade2ca70-3891-5945-98fb-dc099432e06a" @@ -12,9 +12,9 @@ TestItemRunner = "f8b46487-2199-4994-9208-9a1283c18c0a" Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" [compat] -julia = "1.3" +julia = "1.10" DataValues = "0.4.13, 0.5, 1" -ReadStat_jll = "1.1.1" +ReadStat_jll = "1.1.9" [targets] test = ["Test", "TestItemRunner"] From ec658bda240472629962249ab2dd4990aa6017db Mon Sep 17 00:00:00 2001 From: David Anthoff Date: Sun, 30 Aug 2026 23:02:55 -0700 Subject: [PATCH 2/4] Read alignments from the right accessor and unify the type mapping get_alignment called readstat_variable_get_alignment's neighbour, readstat_variable_get_measure, so ReadStatDataFrame.alignments was silently a copy of .measures. Every fixture reports measure UNKNOWN, so the .dta and .xpt files were reporting all-zero alignments where readstat actually says RIGHT. The correct accessor was already wrapped in C_interface.jl but never called. The two get_type methods also disagreed about READSTAT_TYPE_STRING_REF (6): get_type(::Value) mapped it to String, while get_type(::Cint) had no branch for it and fell through to Nothing. That disagreement is not reachable today -- the dta reader rewrites STRING_REF to STRING both when it initialises the variable and when it emits each value, and no other reader mentions the type at all -- but it is worth removing rather than leaving as a trap. Both methods now share one lookup table, which is also where the enum constants pick up their modern spelling: upstream renamed CHAR to INT8 and LONG_STRING to STRING_REF, leaving the values unchanged. Co-Authored-By: Claude Opus 5 --- src/ReadStat.jl | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/src/ReadStat.jl b/src/ReadStat.jl index a0e5e1e..3b5c406 100644 --- a/src/ReadStat.jl +++ b/src/ReadStat.jl @@ -21,12 +21,17 @@ export ReadStatDataFrame, read_dta, read_sav, read_por, read_sas7bdat, read_xpor ############################################################################## const READSTAT_TYPE_STRING = Cint(0) -const READSTAT_TYPE_CHAR = Cint(1) +const READSTAT_TYPE_INT8 = Cint(1) const READSTAT_TYPE_INT16 = Cint(2) const READSTAT_TYPE_INT32 = Cint(3) const READSTAT_TYPE_FLOAT = Cint(4) const READSTAT_TYPE_DOUBLE = Cint(5) -const READSTAT_TYPE_LONG_STRING = Cint(6) +const READSTAT_TYPE_STRING_REF = Cint(6) + +# Julia type for each readstat_type_t, indexed by the enum value plus one. +# STRING_REF is a reference into a string table, so it surfaces as a String +# just like STRING does. +const READSTAT_TYPES = (String, Int8, Int16, Int32, Float32, Float64, String) const READSTAT_ERROR_OPEN = Cint(1) const READSTAT_ERROR_READ = Cint(2) @@ -114,29 +119,14 @@ function get_format(var::Ptr{Nothing}) ptr == C_NULL ? "" : unsafe_string(ptr) end -function get_type(data_type::Cint) - if data_type == READSTAT_TYPE_STRING - return String - elseif data_type == READSTAT_TYPE_CHAR - return Int8 - elseif data_type == READSTAT_TYPE_INT16 - return Int16 - elseif data_type == READSTAT_TYPE_INT32 - return Int32 - elseif data_type == READSTAT_TYPE_FLOAT - return Float32 - elseif data_type == READSTAT_TYPE_DOUBLE - return Float64 - end - return Nothing -end +get_type(data_type::Cint) = READSTAT_TYPES[data_type + 1] get_type(variable::Ptr{Nothing}) = get_type(readstat_variable_get_type(variable)) get_storagewidth(variable::Ptr{Nothing}) = readstat_variable_get_storage_width(variable) get_measure(variable::Ptr{Nothing}) = readstat_variable_get_measure(variable) -get_alignment(variable::Ptr{Nothing}) = readstat_variable_get_measure(variable) +get_alignment(variable::Ptr{Nothing}) = readstat_variable_get_alignment(variable) function handle_variable!(var_index::Cint, variable::Ptr{Nothing}, val_label::Cstring, ds_ptr::Ptr{ReadStatDataFrame}) @@ -165,10 +155,7 @@ function handle_variable!(var_index::Cint, variable::Ptr{Nothing}, return Cint(0) end -function get_type(val::Value) - data_type = readstat_value_type(val) - return [String, Int8, Int16, Int32, Float32, Float64, String][data_type + 1] -end +get_type(val::Value) = get_type(readstat_value_type(val)) Base.convert(::Type{Int8}, val::Value) = ccall((:readstat_int8_value, libreadstat), Int8, (Value,), val) Base.convert(::Type{Int16}, val::Value) = ccall((:readstat_int16_value, libreadstat), Int16, (Value,), val) From 581de66cc6762242cbd246a1ad1ee9123224613f Mon Sep 17 00:00:00 2001 From: David Anthoff Date: Sun, 30 Aug 2026 23:03:12 -0700 Subject: [PATCH 3/4] Correct ccall signatures and drop the dead info handler The four readstat_set_*_handler calls declared a return type of Int, which is 64 bits, where C returns readstat_error_t, a 4-byte enum. The values are discarded, so this was harmless in practice, but the declaration was wrong. readstat_get_modified_time returns time_t, which is signed. Declaring it UInt turned any pre-1970 timestamp into a huge positive number that unix2datetime then choked on. handle_info! was compiled into a cfunction on every parse and then never registered: readstat dropped readstat_set_info_handler long ago, and the metadata handler already sets rows and columns. Co-Authored-By: Claude Opus 5 --- src/C_interface.jl | 2 +- src/ReadStat.jl | 16 ++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/C_interface.jl b/src/C_interface.jl index 4880bcd..eeede3a 100644 --- a/src/C_interface.jl +++ b/src/C_interface.jl @@ -4,7 +4,7 @@ function readstat_get_file_label(metadata::Ptr{Nothing}) end function readstat_get_modified_time(metadata::Ptr{Nothing}) - return ccall((:readstat_get_modified_time, libreadstat), UInt, (Ptr{Nothing},), metadata) + return ccall((:readstat_get_modified_time, libreadstat), Int64, (Ptr{Nothing},), metadata) end function readstat_get_file_format_version(metadata::Ptr{Nothing}) diff --git a/src/ReadStat.jl b/src/ReadStat.jl index 3b5c406..2d26bbf 100644 --- a/src/ReadStat.jl +++ b/src/ReadStat.jl @@ -90,13 +90,6 @@ include("C_interface.jl") ## ############################################################################## -function handle_info!(obs_count::Cint, var_count::Cint, ds_ptr::Ptr{ReadStatDataFrame}) - ds = unsafe_pointer_to_objref(ds_ptr) - ds.rows = obs_count - ds.columns = var_count - return Cint(0) -end - function handle_metadata!(metadata::Ptr{Nothing}, ds_ptr::Ptr{ReadStatDataFrame}) ds = unsafe_pointer_to_objref(ds_ptr) ds.filelabel = readstat_get_file_label(metadata) @@ -252,15 +245,14 @@ end function Parser() parser = ccall((:readstat_parser_init, libreadstat), Ptr{Nothing}, ()) - info_fxn = @cfunction(handle_info!, Cint, (Cint, Cint, Ptr{ReadStatDataFrame})) meta_fxn = @cfunction(handle_metadata!, Cint, (Ptr{Nothing}, Ptr{ReadStatDataFrame})) var_fxn = @cfunction(handle_variable!, Cint, (Cint, Ptr{Nothing}, Cstring, Ptr{ReadStatDataFrame})) val_fxn = @cfunction(handle_value!, Cint, (Cint, Ptr{Nothing}, ReadStatValue, Ptr{ReadStatDataFrame})) label_fxn = @cfunction(handle_value_label!, Cint, (Cstring, Value, Cstring, Ptr{ReadStatDataFrame})) - ccall((:readstat_set_metadata_handler, libreadstat), Int, (Ptr{Nothing}, Ptr{Nothing}), parser, meta_fxn) - ccall((:readstat_set_variable_handler, libreadstat), Int, (Ptr{Nothing}, Ptr{Nothing}), parser, var_fxn) - ccall((:readstat_set_value_handler, libreadstat), Int, (Ptr{Nothing}, Ptr{Nothing}), parser, val_fxn) - ccall((:readstat_set_value_label_handler, libreadstat), Int, (Ptr{Nothing}, Ptr{Nothing}), parser, label_fxn) + ccall((:readstat_set_metadata_handler, libreadstat), Cint, (Ptr{Nothing}, Ptr{Nothing}), parser, meta_fxn) + ccall((:readstat_set_variable_handler, libreadstat), Cint, (Ptr{Nothing}, Ptr{Nothing}), parser, var_fxn) + ccall((:readstat_set_value_handler, libreadstat), Cint, (Ptr{Nothing}, Ptr{Nothing}), parser, val_fxn) + ccall((:readstat_set_value_label_handler, libreadstat), Cint, (Ptr{Nothing}, Ptr{Nothing}), parser, label_fxn) return parser end From dacf96297696d96aa1a6ff4779333df7d9a05bf2 Mon Sep 17 00:00:00 2001 From: David Anthoff Date: Sun, 30 Aug 2026 23:06:42 -0700 Subject: [PATCH 4/4] Test that alignments are read from the alignment accessor Pins the readstat_alignment_t values each fixture actually carries. The .dta and .xpt files report RIGHT (and LEFT for the xpt string column) while every fixture reports measure UNKNOWN, so reading the wrong accessor yields all zeros and fails these assertions. Verified by reverting the fix: the test item fails, and passes again once restored. Also assert no column type comes back as Nothing, which is what a gap in the readstat_type_t mapping would look like. Co-Authored-By: Claude Opus 5 --- test/test_readstat.jl | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/test/test_readstat.jl b/test/test_readstat.jl index f5a041f..d9fcee6 100644 --- a/test/test_readstat.jl +++ b/test/test_readstat.jl @@ -1,11 +1,13 @@ @testitem "ReadStat" begin using DataValues - @testset "ReadStat: $ext files" for (reader, ext) in - ((read_dta, "dta"), - (read_sav, "sav"), - (read_sas7bdat, "sas7bdat"), - (read_xport, "xpt")) + # Expected alignments are readstat_alignment_t values: + # 0 = UNKNOWN, 1 = LEFT, 2 = CENTER, 3 = RIGHT. + @testset "ReadStat: $ext files" for (reader, ext, alignments) in + ((read_dta, "dta", Int32[3, 3, 3, 3, 3, 3]), + (read_sav, "sav", Int32[0, 0, 0, 0, 0, 0]), + (read_sas7bdat, "sas7bdat", Int32[0, 0, 0, 0, 0, 0]), + (read_xport, "xpt", Int32[3, 3, 3, 3, 3, 1])) dtafile = joinpath(@__DIR__, "types.$ext") rsdf = reader(dtafile) @@ -19,5 +21,15 @@ @test data[4] == DataValueArray{Int16}([2, 7, NA]) @test data[5] == DataValueArray{Int8}([2, 7., NA]) @test data[6] == DataValueArray{String}(["2", "7", ""]) + + # Alignments must come from readstat_variable_get_alignment, not from + # the measure accessor sitting next to it in the C API. Every fixture + # reports measure UNKNOWN, so reading the wrong one yields all zeros. + @test rsdf.alignments == alignments + @test rsdf.measures == Int32[0, 0, 0, 0, 0, 0] + + # Every readstat_type_t the readers emit must map to a concrete Julia + # type; a gap in that mapping used to surface as Nothing. + @test all(!=(Nothing), rsdf.types) end end