Skip to content
New issue

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

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

Already on GitHub? # to your account

Patch/strict model #318

Merged
merged 11 commits into from
Jul 24, 2023
Merged

Patch/strict model #318

merged 11 commits into from
Jul 24, 2023

Conversation

anngvu
Copy link
Collaborator

@anngvu anngvu commented Jul 13, 2023

  • Todo - add new tests.

The theme of this PR is enforcing a stricter model, relevant to #118 and #277.

@cconrad8 main validator bc:

Issues that I've noticed (which I think is due to some unclear schematic logic):

  • Making some fields required is actually not working properly. For example it is working with individualID, but not libraryPreparationMethod as seen in validation tests below and if you take a look at an example manifest from the generation test (highlighted columns). I think we can merge this and see if updating to the latest schematic version fixes this issue or it is because of how we're using Depends On to pull in dependent fields.

@anngvu anngvu force-pushed the patch/strict-model branch from 46dd6da to 49652b2 Compare July 14, 2023 16:07
@github-actions
Copy link

Test Suite Report

Template Generation

template result link
ClinicalAssayTemplate 😄 template link
EpigeneticsAssayTemplate 😄 template link
FlowCytometryTemplate 😄 template link
GenomicsAssayTemplate 😄 template link
ImagingAssayTemplate 😄 template link
LightScatteringAssayTemplate 😄 template link
MRIAssayTemplate 😄 template link
PharmacokineticsAssayTemplate 😄 template link
PlateBasedReporterAssayTemplate 😄 template link
ProcessedAlignedReadsTemplate 😄 template link
ProcessedExpressionTemplate 😄 template link
ProcessedVariantCallsTemplate 😄 template link
ProteomicsAssayTemplate 😄 template link
ProtocolTemplate 😄 template link
RNASeqTemplate 😄 template link
ScRNASeqTemplate 😄 template link
UpdateMilestoneReport 😄 template link
WESTemplate 😄 template link
WGSTemplate 😄 template link

Manifest Validation

manifest result expectation
GenomicsAssayTemplate_0.csv 😄 Lists can be blank if attr not required using ‘list like’ rule
GenomicsAssayTemplate_1.csv 😄 Single list val works by using ‘list like’ rule
GenomicsAssayTemplate_2.csv 😄 Mixing blanks and regular list values works
GenomicsAssayTemplate_3.csv 😄 Conditional validation for attributes is currently not supported
GenomicsAssayTemplate_4.csv A (double) value of 1.5 fails with rule int::inRange 1 2
GenomicsAssayTemplate_5.csv A value of 0 (outside range) fails with rule int::inRange 1 2
RNASeqTemplate_0.csv 😄 Pass as this is the ‘control’ good manifest
RNASeqTemplate_1.csv 😄 Fail because of missing data in required field individualID
RNASeqTemplate_2.csv Fail because of missing data in required field libraryPreparationMethod

@anngvu
Copy link
Collaborator Author

anngvu commented Jul 18, 2023

Update: found an issue that maybe explains why some required columns are not highlighted blue: Sage-Bionetworks/schematic#1018

Copy link
Contributor

@cconrad8 cconrad8 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for making the changes

@anngvu anngvu merged commit 9913c25 into main Jul 24, 2023
@anngvu anngvu deleted the patch/strict-model branch July 24, 2023 23:30
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
3 participants