Skip to content

240 architecture implement quality control pipeline orchestration - #243

Merged
roryclaydon1994 merged 10 commits into
developfrom
240-architecture-implement-quality-control-pipeline-orchestration
Aug 27, 2026
Merged

240 architecture implement quality control pipeline orchestration#243
roryclaydon1994 merged 10 commits into
developfrom
240-architecture-implement-quality-control-pipeline-orchestration

Conversation

@jeipollack

@jeipollack jeipollack commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR implement a minimal quality control pipeline orchestrator; it builds the processing sequence for resource resolution, metric computation, and policy application.

Closes #240 #246

What’s changed

  • Add QualityControlContext class definition
  • Define QualityControlResult for metric results and rejection masks
  • Add helpers to instantiate enabled metrics and rejection policies
  • Add value attribute to ThresholdRejectionPolicy
  • Build the processing sequence for resource resolution, metric computation, and policy application
  • Add unit and integration tests for the orchestration

How to test / verify

  • Check CI tests pass.
  • Confirm the integration test covers resource resolution, metric computation, and rejection-mask generation.

Scope

Indicate the type of PR:

  • Feature
  • Bug fix
  • Hotfix
  • Documentation / process change
  • Internal / refactor
  • Release

Changelog

Did this PR introduce user-visible changes?
If yes, a Scriv changelog fragment must be added and committed.

  • Changelog fragment added (if applicable)

The quality control pipeline interface and configuration are still being finalised. A changelog fragment will be added with the PR that finalises the user-facing pipeline behaviour.

Reviewer Checklist

Reviewers should confirm the following before approving and merging:

  • The PR targets the correct base branch (develop, or main for release PRs)
  • The PR is assigned to the developer
  • Appropriate labels are applied
  • The PR is included in relevant projects and/or milestones
  • Description clearly explains what has changed
  • Issue references included, if applicable
  • Code and documentation adhere to current standards (ruff)
  • Documentation updates included, if relevant
  • CI tests are passing
  • All reviewer comments have been addressed

Next Steps / Notes (if applicable)

The next step is to implement preparation of missing resources so that the resource resolution stage can provide all resources required by enabled quality metrics.

@jeipollack jeipollack self-assigned this Aug 18, 2026
@jeipollack
jeipollack force-pushed the 240-architecture-implement-quality-control-pipeline-orchestration branch from 89a896b to ae8b21b Compare August 18, 2026 14:13
@jeipollack
jeipollack force-pushed the 240-architecture-implement-quality-control-pipeline-orchestration branch 2 times, most recently from 1c43b37 to b64ea75 Compare August 21, 2026 09:52
@jeipollack
jeipollack force-pushed the 240-architecture-implement-quality-control-pipeline-orchestration branch from b64ea75 to 34b408d Compare August 25, 2026 15:19
Jennifer Pollack added 6 commits August 26, 2026 14:21
- Initialize the MetricsRegistry in the pipeline constructor
- Instantiate enabled quality metric implementations from configuration
- Add unit tests for valid and invalid metric configurations
- Add helpers to instantiate enabled metrics and rejection policies
- Build the processing sequence for resource resolution, metric computation, and policy application
- Add unit and integration tests for the orchestration
- Clean up whitespace in the quality control configuration fixture
@jeipollack
jeipollack force-pushed the 240-architecture-implement-quality-control-pipeline-orchestration branch from 97f575e to c0b7954 Compare August 26, 2026 12:22
@jeipollack jeipollack added this to the Outlier Removal milestone Aug 26, 2026
@jeipollack
jeipollack marked this pull request as ready for review August 26, 2026 12:37

@roryclaydon1994 roryclaydon1994 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Feedback

Very complete, great tests and design.

I think in terms of design, I would suggest splitting out the instantiate components of the run method of QualityControlPipeline so that run only has the responsibility of running and all loading and resolving responsibilities are in a separate method.

I would also suggest sprinkling in some logging statements as you see fit to help observability.

Comment thread src/wf_psf/quality_control/context.py
Comment thread src/wf_psf/quality_control/pipeline.py
Comment thread src/wf_psf/quality_control/pipeline.py Outdated
Comment thread src/wf_psf/quality_control/pipeline.py
Comment thread src/wf_psf/quality_control/pipeline.py Outdated
Comment thread src/wf_psf/quality_control/pipeline.py Outdated
Comment thread src/wf_psf/tests/test_quality_control/pipeline_test.py Outdated
Comment thread src/wf_psf/quality_control/pipeline.py
Jennifer Pollack added 4 commits August 27, 2026 10:59
- Rename rejection_masks to validity_masks
- Define boolean validity mask in RejectionPolicy apply method doc string
- Add pipeline integration test and fixture for multiple rejection policies
- Rename test test_pipeline_run to test_pipeline_run_single_rejection_policy
@jeipollack

Copy link
Copy Markdown
Contributor Author

Thanks a mill, @roryclaydon1994! I addressed each comment and made some updates. I also corrected some syntax errors in the public quality_control.yaml file which I discovered during my dev meeting with my colleagues.

@roryclaydon1994
roryclaydon1994 merged commit 8821f07 into develop Aug 27, 2026
2 checks passed
@roryclaydon1994
roryclaydon1994 deleted the 240-architecture-implement-quality-control-pipeline-orchestration branch August 27, 2026 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

2 participants