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

make xpt v8 SAS-readable #207

Merged
merged 4 commits into from Aug 25, 2020
Merged

make xpt v8 SAS-readable #207

merged 4 commits into from Aug 25, 2020

Conversation

reikoch
Copy link
Contributor

@reikoch reikoch commented Aug 6, 2020

Hi Evan!

I propose very few changes to make xpt version 8 files SAS-readable. On some examples ReadStat now produces readable files by setting format and informat name to blank (not nulls).

Given https://documentation.sas.com/?docsetId=movefile&docsetTarget=p0ld1i106e1xm7n16eefi7qgj8m9.htm&docsetVersion=9.4&locale=en I would be willing to improve on this PR with target to preserve also long variable names, formats, and informats. Would that be of interest to you?

@evanmiller
Copy link
Contributor

Hi, thank you for the contribution. Additional fixes to improve compatibility with SAS would be very welcome.

@reikoch
Copy link
Contributor Author

reikoch commented Aug 7, 2020

Will do in the next days!

@evanmiller
Copy link
Contributor

When you do, please point the PR at the dev branch rather than master. Thanks.

@reikoch reikoch changed the base branch from master to dev August 11, 2020 13:13
@evanmiller
Copy link
Contributor

Is this ready for merge?

@reikoch
Copy link
Contributor Author

reikoch commented Aug 21, 2020

I want to preserve dates and datetimes,
but current PR certainly does not destroy anything

@evanmiller
Copy link
Contributor

Okay, I'm not in a rush so I'll leave it open for now. Let me know when you're happy with it.

Copy link
Contributor

@evanmiller evanmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to assign the return values of readstat_convert and xport_construct_format to retval, leaving in the goto statements. I think that was the original intention of the code.

@evanmiller
Copy link
Contributor

Well, this is what I had in mind: eb391f6

@reikoch
Copy link
Contributor Author

reikoch commented Aug 23, 2020

This is how good it gets right now. Table labels are not written to xpt v8 files, that is not top priority. Table labels are not read or written for SAS datasets. reading sas7bdat is reading table name but declares it file_label in ctx. Example dataset dates.sas7bdat in https://github.com/reikoch/testfiles display file_label 'dataset label for the dates test dataset just before variable formats are stored.

Ready for merge if OK; can help with table label if so wished.

@evanmiller
Copy link
Contributor

The automated tests indicate a segmentation fault. This will need to be fixed before merging.

@reikoch
Copy link
Contributor Author

reikoch commented Aug 25, 2020

OK Evan,
let's try to get this version merged in. It creates SAS-readable xpt v8 files, though table name and dates/times/datetimes specifications are dropped.

@evanmiller evanmiller merged commit 73bb8a7 into WizardMac:dev Aug 25, 2020
@evanmiller
Copy link
Contributor

Done! Feel free to open other PRs with additional fixes.

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.

None yet

2 participants