Skip to content

Pin ReadStat_jll to 1.1.9, raise Julia floor to 1.10, fix wrapper defects - #95

Merged
davidanthoff merged 4 commits into
mainfrom
readstat-jll-1.1.9-and-wrapper-fixes
Aug 31, 2026
Merged

davidanthoff merged 4 commits into
mainfrom
readstat-jll-1.1.9-and-wrapper-fixes

Conversation

@davidanthoff

Copy link
Copy Markdown
Member

Audit of what binary we actually ship, plus the wrapper defects it turned up.

We were not shipping the latest binary

ReadStat_jll 1.1.8 and newer require julia 1.6, but our julia floor was 1.3 and our bound was ReadStat_jll = "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 instead of 1.1.9.

Pinned to "1.1.9" with julia = "1.10", matching Query.jl and ReadStatTables.jl. Raising the floor is breaking for old-julia users, so the version goes to 1.2.0-DEV.

For the record on the binary itself: 1.1.9 (2023-02-20) is still upstream's newest release, so we are now on the latest tag that exists. Upstream dev is 58 commits ahead and most other bindings (pyreadstat, haven, readstat-rs) have moved to pinned dev commits, but this PR deliberately stays on the released tag. Asking upstream to tag is being handled separately on WizardMac/ReadStat#343.

Wrapper defects

  • Alignments were read from the wrong accessor. get_alignment called readstat_variable_get_measure, so ReadStatDataFrame.alignments was silently a copy of .measures. Every fixture reports measure UNKNOWN, so .dta and .xpt 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 disagreed about READSTAT_TYPE_STRING_REF: one mapped it to String, the other fell through to Nothing. Not reachable today - the dta reader rewrites STRING_REF to STRING before the handlers ever see it, and no other reader mentions the type - so this removes a trap rather than fixing a live bug. Both now share one lookup table, which also picks up the modern enum spelling: upstream renamed CHAR to INT8 and LONG_STRING to STRING_REF, values unchanged.
  • ccall signatures. The four readstat_set_*_handler calls declared Int (64-bit) where C returns a 4-byte enum; harmless only because the values are discarded. readstat_get_modified_time was declared UInt where C returns signed time_t, so a pre-1970 timestamp became a huge positive number that unix2datetime choked on.
  • Dead code. 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.

Testing

The test item now pins the readstat_alignment_t values each fixture carries. Verified the assertion is real by reverting the alignment fix: the item fails, and passes again once restored. Full suite green, and StatFiles' own suite (including its exact-string Show item) passes against this branch.

Known gaps left as follow-up issues: read_por has no test or fixture, and read_xport is exported but StatFiles never wires .xpt up.

🤖 Generated with Claude Code

davidanthoff and others added 4 commits August 30, 2026 23:00
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Aug 31, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.33%. Comparing base (a0acea1) to head (dacf962).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/ReadStat.jl 85.71% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #95      +/-   ##
==========================================
+ Coverage   79.66%   83.33%   +3.67%     
==========================================
  Files           2        2              
  Lines         177      156      -21     
==========================================
- Hits          141      130      -11     
+ Misses         36       26      -10     
Flag Coverage Δ
unittests 83.33% <87.50%> (+3.67%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@davidanthoff
davidanthoff merged commit 775288a into main Aug 31, 2026
31 checks passed
@davidanthoff
davidanthoff deleted the readstat-jll-1.1.9-and-wrapper-fixes branch August 31, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants