1182: Add custom exceptions for unsupported XPT format and enhance error handling in XPTReader#1644
Conversation
…ndling in XPTReader
…into 1182-SAS-V8-Transport-File-Error-Message
RamilCDISC
left a comment
There was a problem hiding this comment.
The PR adds only exception for unsupported xpt format but does not actually verifies the version of the xpt. The AC requires that the version of xpt is validated and if its not v5 then custom exception should be thrown. I tried with ae.xpt file (v8 version from the zip attached in ticket). The validation executed fully and did not throw any exception.
|
@RamilCDISC, I execute with ae.xpt file (v8 version from the zip attached in ticket) and set -l error . |
|
When using pd.read_sas, the argument format="xport" indicates that we expect a SAS Transport V5 file. Therefore, if there is a version mismatch while reading the SAS file, the error "Header record is not an XPORT file." is raised. At the moment, we catch all possible errors during read_sas. In the next commit, I will narrow this down to catching only this specific error: "Header record is not an XPORT file." I can also add unit tests for attempts to read SAS files of different versions (V5 and V8). If you have examples of test data for other versions as well, please share them and I will include them in the tests. |
|
@alexfurmenkov The v8 xpt test data is attached in the connected issue. You can take from there and v5 xpt are available in the test resources. i have verified, the engine does show error. What we can decide on is do we want for such a file skip execution for all rules or show RULE EXECUTION ERROR in the status column of the report. @SFJohnson24 What do you think? |
|
@RamilCDISC I think skipping will take more of a lift. If a user tries to submit bad data--the execution should be halted until they format in v5. |
|
@SFJohnson24 Thanks for your response. @alexfurmenkov the PR is then correct. Are you still planning to narrow down the error catch as you mentioned in your previous comment, os I should perform final validation and add closing comments. |
…dd tests for XPT v5 and v8
…into 1182-SAS-V8-Transport-File-Error-Message
…thub.com/cdisc-org/cdisc-rules-engine into 1182-SAS-V8-Transport-File-Error-Message
|
Hi @RamilCDISC |
RamilCDISC
left a comment
There was a problem hiding this comment.
The PR adds exception and error handling to xpt_reader class. The error highlights that only v5 transport files are supported only. The validation was done by:
- Reviewing the PR for nay unwanted code or comments.
- Reviewing the PR logic in accordance with AC.
- Ensuring all unit and regression testing pass.
- Ensuring new tests form updated behavior are added.
- Running manual validation using CLI with valid xpt file.
- Running manual validation using CLI with unsupported xpt file.
- Ensuring proper error messages in report.
- Ensuring study based rules still work fine when used with unsupported xpt version.
SFJohnson24
left a comment
There was a problem hiding this comment.
PR correctly addresses issue with incorrect XPT formatting

No description provided.