Skip to content

Add support for the PAGE_METC_TYPE page type#75

Merged
nhirschey merged 1 commit intofsprojects:masterfrom
easymorph:master
Nov 24, 2021
Merged

Add support for the PAGE_METC_TYPE page type#75
nhirschey merged 1 commit intofsprojects:masterfrom
easymorph:master

Conversation

@AndrewRybka
Copy link
Copy Markdown
Contributor

According to the description of the SAS7BDAT format and the comments in this file pages with the PAGE_METC_TYPE type can be processed in the same way as the PAGE_META_TYPE pages.

We stumbled upon several files with PAGE_METC_TYPE pages. And after this fix, we were able to import them successfully.

@nhirschey nhirschey closed this Nov 23, 2021
@nhirschey
Copy link
Copy Markdown
Collaborator

Close/reopen to trigger wormow

@nhirschey nhirschey reopened this Nov 23, 2021
@nhirschey
Copy link
Copy Markdown
Collaborator

Thanks for this contribution! It would be great if you could add a test to cover this page type.

Do you know of an online example file with this file page type that you would be able to add to the unit tests? The tests are done in https://github.com/fsprojects/FSharp.Data.Toolbox/blob/master/tests/FSharp.Data.Toolbox.Sas.Tests/Integration.fs, and the csv file in that folder has the urls of files that get downloaded and tested.

It’s possible that some of the files that are being skipped in the ‘failingfiles’ set in integration.fs have this meta type. If so, then you could use one of those as the test files rather than locating a new one.

@AndrewRybka
Copy link
Copy Markdown
Contributor Author

Unfortunately, the files that I mentioned above are confidential. So I can't share them.

As for the files that are being skipped in the integration.fs file - most of them are no longer available for download.
And those that are available result in a different error.

I tried to find files to replicate this issue on the Internet, but with no luck.

@nhirschey
Copy link
Copy Markdown
Collaborator

Ok, thanks for checking the failing files. Given the small change and the great documentation of the issue that you provided let’s go ahead and merge this in. It will show up in the nuget.org feed shortly. Let me know if you find any issues with it.

Thanks again!

@nhirschey nhirschey merged commit 5d55b46 into fsprojects:master Nov 24, 2021
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