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

QC Report branch with test refinement #1428

Merged
merged 60 commits into from
Jan 16, 2024

Conversation

@turbomam turbomam linked an issue Nov 28, 2023 that may be closed by this pull request
@turbomam turbomam changed the title works because everything is commented out Mark's branch of QC Report branch Nov 28, 2023
@turbomam turbomam changed the title Mark's branch of QC Report branch QC Report branch with test refinement Nov 28, 2023
@turbomam
Copy link
Member Author

turbomam commented Dec 5, 2023

@brynnz22 @aclum can we just remove the rule and the invalid examples? It looks like it will build then.

@aclum
Copy link
Contributor

aclum commented Dec 5, 2023

@turbomam I will test that now.

aclum added 2 commits December 5, 2023 15:02
Removing rule to have has_output be optional if quality_control_report.status=fail
Testing removing invalid data of quality_control_report.status=pass mixed with failure_categorization.
@aclum
Copy link
Contributor

aclum commented Dec 5, 2023

@turbomam I need some help getting the mixs.yaml conflicts resolved.

@turbomam
Copy link
Member Author

turbomam commented Dec 5, 2023

You can delete it from the branch. It gets regenerated by make all, if necessary.

Or I can do it in about two hours.

@aclum
Copy link
Contributor

aclum commented Dec 5, 2023

@turbomam the build is failing b/c the example valid data is missing has_output. I thought you were going to add other tests/change how the model was done so this data would pass.

snakecasing permissible values for FailureWhatEnum
@aclum aclum linked an issue Dec 5, 2023 that may be closed by this pull request
aclum added 2 commits December 5, 2023 15:47
Updating failure_what values to snakecase permissible values
Updating failure_where value to snakecase permissible value.
@turbomam
Copy link
Member Author

turbomam commented Dec 6, 2023

oh yeah. A reasonable solution would be to assert recommended: true instead of required: true on has_output's slot_usage in WorkflowExecutionActivity. I would especially want @scanon or someone else he designates to sign off on that change.

aclum added 7 commits December 6, 2023 15:55
Adding quality control slots to PlannedProcess
Adding quality control slots to WorkflowExeuctionActivity
Updates to how quality control slots are modeled
Updates to how quality control information is modeled.
Update example valid data for different quality control modeling.
Updates to modeling for quality control slots
Making qc_failure_what and qc_failure_where multivalued.
Add new test for rule to test that has_output is required if qc_status is not specified.
@aclum aclum linked an issue Dec 21, 2023 that may be closed by this pull request
@aclum
Copy link
Contributor

aclum commented Dec 21, 2023

To get the value_presence rules to work correctly I had to update linkml to 1.6.6, then @pkalita-lbl helped identify an issue where rules don't inherit to subclasses. Patrick will make a ticket for the linkml issue, this should be released in early January. Workaround would be to copy rules to each subclass. cc @turbomam @brynnz22

@pkalita-lbl
Copy link
Collaborator

LinkML issue: linkml/linkml#1802

@aclum aclum requested a review from brynnz22 January 12, 2024 20:42
src/schema/nmdc.yaml Show resolved Hide resolved
src/schema/nmdc.yaml Show resolved Hide resolved
src/schema/workflow_execution_activity.yaml Outdated Show resolved Hide resolved
src/schema/workflow_execution_activity.yaml Show resolved Hide resolved
src/schema/workflow_execution_activity.yaml Outdated Show resolved Hide resolved
aclum and others added 5 commits January 12, 2024 15:49
Add comment about permissible values for FailureWhereEnum
Deleting commented out rule
remove having failure_what and failure_where be multivalued, with current modeling failure_categorization is what is multivalued with slots for failure_what and failure_where
@aclum aclum merged commit 03e267e into main Jan 16, 2024
2 checks passed
@aclum aclum deleted the 1427-try-to-fix-qc-report-pr-1334-in-new-branch branch January 16, 2024 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants