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.

Sign up for GitHub

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

added pre-qc-failed status filter #24

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

added pre-qc-failed status filter #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4dea336
added pre-qc-failed status filter
KateSakharova Dec 17, 2024
32ec2e8
added default for PRE_QC status
KateSakharova Dec 17, 2024
3654927
Merge branch 'main' into fix/add-pre-qc-failed-filter
mberacochea Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion analyses/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ def default_status(cls):
cls.ASSEMBLY_UPLOADED: False,
cls.ASSEMBLY_UPLOAD_FAILED: False,
cls.ASSEMBLY_UPLOAD_BLOCKED: False,
cls.PRE_ASSEMBLY_QC_FAILED: False,
Copy link
Member

@mberacochea mberacochea Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Please put "cls.PRE_ASSEMBLY_QC_FAILED: False," after "cls.ASSEMBLY_STARTED: False," :)

}

status = models.JSONField(
Expand Down Expand Up @@ -504,7 +505,7 @@ class AnalysisStates(str, Enum):
ANALYSIS_COMPLETED = "analysis_completed"
ANALYSIS_BLOCKED = "analysis_blocked"
ANALYSIS_FAILED = "analysis_failed"
ANALYSIS_QC_FAILED = "analysis_qc_failed"
ANALYSIS_PRE_QC_FAILED = "analysis_pre_qc_failed"
Copy link
Member

Choose a reason for hiding this comment

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

For analyses we don't have a "pre-qc".. it's just QC, right?

ANALYSIS_POST_SANITY_CHECK_FAILED = "analysis_post_sanity_check_failed"

@classmethod
Expand All @@ -514,6 +515,7 @@ def default_status(cls):
cls.ANALYSIS_COMPLETED: False,
cls.ANALYSIS_BLOCKED: False,
cls.ANALYSIS_FAILED: False,
cls.ANALYSIS_PRE_QC_FAILED: False,
}

status = models.JSONField(
Expand Down
3 changes: 2 additions & 1 deletion workflows/flows/analysis_amplicon_study.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def get_analyses_to_attempt(
**{
f"status__{analyses.models.Analysis.AnalysisStates.ANALYSIS_COMPLETED}": False,
f"status__{analyses.models.Analysis.AnalysisStates.ANALYSIS_BLOCKED}": False,
f"status__{analyses.models.Analysis.AnalysisStates.ANALYSIS_PRE_QC_FAILED}": False,
}
)
.filter(experiment_type=for_experiment_type)
Expand Down Expand Up @@ -546,7 +547,7 @@ def set_post_analysis_states(amplicon_current_outdir: Path, amplicon_analyses: L
if analysis.run.first_accession in qc_failed_runs:
task_mark_analysis_status(
analysis,
status=analyses.models.Analysis.AnalysisStates.ANALYSIS_FAILED,
status=analyses.models.Analysis.AnalysisStates.ANALYSIS_PRE_QC_FAILED,
reason=qc_failed_runs[analysis.run.first_accession],
)
elif analysis.run.first_accession in qc_completed_runs:
Expand Down
1 change: 1 addition & 0 deletions workflows/flows/assemble_study.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def get_assemblies_to_attempt(study: analyses.models.Study) -> List[Union[str, i
**{
f"status__{analyses.models.Assembly.AssemblyStates.ASSEMBLY_COMPLETED}": False,
f"status__{analyses.models.Assembly.AssemblyStates.ASSEMBLY_BLOCKED}": False,
f"status__{analyses.models.Assembly.AssemblyStates.PRE_ASSEMBLY_QC_FAILED}": False,
Copy link
Member

Choose a reason for hiding this comment

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

This one should go first; it doesn't change how it works—just improves readability

Copy link
Member

Choose a reason for hiding this comment

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

A little manager mixin helper would be nice for building these more readably:

class SelectByStatusManagerMixin:
    STATUS_FIELDNAME = "status"
    
    def filter_by_statuses(self, statuses_to_be_true: List[Union[str, Enum]] = None, statuses_to_be_false: List[Union[str, Enum]] = None):
        """
        Filter queryset by a combination of statuses in the objects status json field.
        """
        filters = {}
        if statuses_true:
            filters = {
                f"{self.STATUS_FIELDNAME}__{must_be}": True
                for must_be in statuses_true
            }
        if statuses_false:
            filters.update({
                f"{self.STATUS_FIELDNAME}__{must_not_be}": True
                for must_not_be in statuses_false
            })
        return super().get_queryset().filter(**filters)

Then we can do class AssemblyManager(SelectByStatusManagerMixin, ENADerivedManager): ...

and select more readably, like

assemblies_worth_trying = study.assemblies_reads.filter_by_statuses(
    statuses_to_be_false=[
        analyses.models.Assembly.AssemblyStates.ASSEMBLY_COMPLETED,
        analyses.models.Assembly.AssemblyStates.ASSEMBLY_BLOCKED,
        analyses.models.Assembly.AssemblyStates.PRE_ASSEMBLY_QC_FAILED,
    ]
).values_list("id", flat=True)

(or maybe better yet include an exclude_by_statuses chainable method on the mixin).

Happy to add this later as it will need a couple of new tests. Just documenting whilst it came to mind!

}
).values_list("id", flat=True)
return assemblies_worth_trying
Expand Down
2 changes: 1 addition & 1 deletion workflows/tests/test_analysis_amplicon_study_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ async def test_prefect_analyse_amplicon_flow(
# check failed runs
assert (
await analyses.models.Analysis.objects.filter(
status__analysis_failed=True
status__analysis_pre_qc_failed=True
).acount()
== 1
)
Expand Down
Loading