Skip to content

changes in validation script to change engine location#22

Open
alexfurmenkov wants to merge 28 commits into
mainfrom
rules_2
Open

changes in validation script to change engine location#22
alexfurmenkov wants to merge 28 commits into
mainfrom
rules_2

Conversation

@alexfurmenkov
Copy link
Copy Markdown
Collaborator

No description provided.

@alexfurmenkov alexfurmenkov marked this pull request as ready for review May 13, 2026 12:02
tr,TRSTAT,Completion Status,Char,50
tr,TRREASND,Reason Not Done,Char,50
tr,TRNAM,Laboratory/Vendor Name,Char,50
tr,TRLOC,Location of the Tumor/Lesion,Char,50
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here in the data TRLOC is added. The relevant rule in this folder checks for presence of this variable and raises error if this is present. The result.csv file is not updated which I believe would change after addition of this variable in the data.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

TRLOC should not be in the positive data-- it should only exist in the negative data as that is the failure criteria for the rule


done # cases
done # test types
done < <(find "$TYPE_DIR" -mindepth 1 -maxdepth 1 -type d -print0 | sort -z)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is for to handle the spaces in the path safely but the for loop in the top does not use it. For this to be affective use while loop instead of for loop.

@alexfurmenkov
Copy link
Copy Markdown
Collaborator Author

@SFJohnson24 or anyone who can, can you please delete this PR and rules_2 branch fromBranches? rules_2 became protected and I can't push anymore. created new PR with the same changes and latest changes requested by @RamilCDISC

@SFJohnson24
Copy link
Copy Markdown
Collaborator

@alexfurmenkov resolved the branch protections issue--I believe you shouldnt have a problem now

@alexfurmenkov alexfurmenkov requested a review from RamilCDISC May 21, 2026 08:15
Copy link
Copy Markdown

@RamilCDISC RamilCDISC left a comment

Choose a reason for hiding this comment

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

Only this small change after which I think it would be good to merge.

R_MATCH="$4" \
R_DIFF="$5" \
R_STDERR="$6" \
python3 -c "
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we should not hardcode python3 here. The script may fail on system where python is not registered as python3

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.

3 participants