sanger / limber

A config-driven LIMS built on Sequencescape, primarily for running library preparation pipelines in the laboratory
MIT License
3 stars 8 forks source link

Y24-123: Cell counting - QC labels - quick fix #1736

Closed StephenHulme closed 3 months ago

StephenHulme commented 3 months ago

Closes #1729

Changes proposed in this pull request

Instructions for Reviewers

[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to main]
    - Check story numbers included
    - Check for debug code
    - Check version

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.26%. Comparing base (ed1e7a2) to head (2f93ffd).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #1736 +/- ## =========================================== + Coverage 91.24% 91.26% +0.02% =========================================== Files 368 369 +1 Lines 7572 7581 +9 =========================================== + Hits 6909 6919 +10 + Misses 663 662 -1 ``` | [Flag](https://app.codecov.io/gh/sanger/limber/pull/1736/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanger) | Coverage Δ | | |---|---|---| | [javascript](https://app.codecov.io/gh/sanger/limber/pull/1736/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanger) | `93.65% <ø> (ø)` | | | [pull_request](https://app.codecov.io/gh/sanger/limber/pull/1736/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanger) | `91.26% <100.00%> (?)` | | | [push](https://app.codecov.io/gh/sanger/limber/pull/1736/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanger) | `91.26% <100.00%> (+0.02%)` | :arrow_up: | | [ruby](https://app.codecov.io/gh/sanger/limber/pull/1736/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanger) | `90.93% <100.00%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanger#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

KatyTaylor commented 3 months ago

Changing this class changes the label printing for the Cardinal pipeline as well as for the new scRNA pipeline (search for use of this label template plate_cellaca_qc), are we Ok with that? They specifically asked for it not to waste labels in that pipeline (labels are surprisingly expensive).

If we are ok with that then it looks good.

Am I right in thinking it would reasonably easy to change it so we're not breaking Cardinal? New plate label subclass, override that method, change purpose config to point to new class...?

If so I think we should do that - even though it looks like the Cardinal pipeline might become obsolete, we don't know that for sure yet so makes sense to not break it.

StephenHulme commented 3 months ago

Thanks for the comments @andrewsparkes and @KatyTaylor, I've created a new class as suggested and updated the configs.

codeclimate[bot] commented 3 months ago

Code Climate has analyzed commit 2f93ffd9 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 90.9% (0.0% change).

View more on Code Climate.