[jamesalster]

PR proposing a fix to issue #52, with a brief test of the file read. I'm new to this repository and the topic, so I may have made some blunders. I haven't worked out if we would ideally read in string columns with appropriate string lengths, nor how to do that.

A file to recreate the issue is added at data/string_labeled_value_eg.sav. Reading this in with haven, and this branch, gives the correct string value => string label mapping, showing the regions of Belgium (in this case). But on main, the regions of Belgium are not read in.

Tests run fine on my machine.

[junyuan-chen]

Well, the reason why value labels over strings are not allowed here (intentionally) is because libreadstat requires the values attached with value labels to be either Int32 or Char from what I remember. It's possible that value labels may be allowed for string columns (in SAS or SPSS), but I will want to take a look at what haven does to make this work.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.63%. Comparing base (f1da846) to head (0a92748).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #53      +/-   ##
==========================================
+ Coverage   99.55%   99.63%   +0.07%     
==========================================
  Files          12       12              
  Lines        1361     1372      +11     
==========================================
+ Hits         1355     1367      +12     
+ Misses          6        5       -1     

☔ 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.

[jamesalster]

Right, I see! Thanks, I'm not familiar with libreadstat. From a user point of view, I would say that handling the string labels is definitely expected behaviour, since they do appear from time to time in mainstream datasets (the file I used there is an excerpt from the European Union's Eurobarometer 2010).

But I of course defer to you on implementation, and I'm happy to contribute more time to implement things another way, if you suggest a direction.

[junyuan-chen]

@jamesalster I realized that what I said earlier is wrong. libreadstat does support reading/writing value labels for string variables. I therefore revived this PR and added the write part.