Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing value support for integers in SPSS #251

Closed
gorcha opened this issue Aug 4, 2021 · 4 comments
Closed

Missing value support for integers in SPSS #251

gorcha opened this issue Aug 4, 2021 · 4 comments

Comments

@gorcha
Copy link
Contributor

gorcha commented Aug 4, 2021

Another one from haven! 🙂
From tidyverse/haven#596.

Currently missing value support for SPSS is limited to doubles and strings because of an explicit type check in the functions that write out the missing values info:

static readstat_error_t sav_n_missing_values(int *out_n_missing_values, readstat_variable_t *r_variable) {
int n_missing_values = 0;
if (r_variable->type == READSTAT_TYPE_DOUBLE) {
n_missing_values = sav_n_missing_double_values(r_variable);
} else if (readstat_variable_get_storage_width(r_variable) <= 8) {
n_missing_values = sav_n_missing_string_values(r_variable);
}
if (abs(n_missing_values) > 3) {
return READSTAT_ERROR_TOO_MANY_MISSING_VALUE_DEFINITIONS;
}
if (out_n_missing_values)
*out_n_missing_values = n_missing_values;
return READSTAT_OK;
}

static readstat_error_t sav_emit_variable_missing_values(readstat_writer_t *writer, readstat_variable_t *r_variable) {
if (r_variable->type == READSTAT_TYPE_DOUBLE) {
return sav_emit_variable_missing_double_values(writer, r_variable);
} else if (readstat_variable_get_storage_width(r_variable) <= 8) {
return sav_emit_variable_missing_string_values(writer, r_variable);
}
return READSTAT_OK;
}

It seems like it should be OK to just use the double methods for all non-string values (since integers and 32 bit floats are all converted to doubles in SPSS output anyway) - it probably just needs a change to the conditional logic in these functions and maybe a couple of additional readstat_add_missing_* functions in here to deal with other types:
https://github.com/WizardMac/ReadStat/blob/a8e6705453f0e610a698a4d98b3dfa3087cf4b28/src/readstat_variable.c

I can chuck together a PR if you're happy with this approach?

@evanmiller
Copy link
Contributor

From your perspective, would it be sufficient to change the type checks to:

if (readstat_variable_get_type_class(r_variable) == READSTAT_TYPE_CLASS_NUMERIC)

?

@gorcha
Copy link
Contributor Author

gorcha commented Aug 4, 2021

Yep that's perfect, thank you!

evanmiller added a commit that referenced this issue Aug 4, 2021
@evanmiller
Copy link
Contributor

Please try the latest dev commits and let me know if that works for you.

@gorcha
Copy link
Contributor Author

gorcha commented Aug 4, 2021

That works perfectly, thanks again

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

No branches or pull requests

2 participants