Skip to content

Comments

Feature/DIMS_QCinfo_in_mail#93

Merged
mraves2 merged 12 commits intodevelopfrom
feature/DIMS_QCinfo_in_mail
Feb 20, 2026
Merged

Feature/DIMS_QCinfo_in_mail#93
mraves2 merged 12 commits intodevelopfrom
feature/DIMS_QCinfo_in_mail

Conversation

@mraves2
Copy link
Contributor

@mraves2 mraves2 commented Jan 16, 2026

Deze feature zorgt ervoor dat er extra QC informatie vanuit de DIMS pipeline in de eindmail komt, zodat de gebruiker in 1 oogopslag de kwaliteit van de run kan beoordelen.
Verschillende stappen van de pipeline, met name AverageTechReplicates en GenerateQCOutput, genereren extra txt bestanden, die als content opgenomen worden in DIMS.nf.

Copy link
Contributor

@BasMonkey BasMonkey left a comment

Choose a reason for hiding this comment

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

I've made some remarks about performance and code duplication. Please see the comments left on the code.

is_selected <- is_pos_selection_subset$HMDB_name[line_index]
thresh_selected <- all_is_thresholds$plasma$pos[which(all_is_thresholds$names$pos == is_selected)]
if (is_pos_selection_subset$Intensity[line_index] < thresh_selected) {
is_below_threshold <- rbind(is_below_threshold, is_pos_selection_subset[line_index, ])
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid rbind() in a loop, since it repeatedly reallocates and copies the data frame, which is inefficient and may use a huge amount of ram for larger datasets. Consider collecting indices or rows first and binding once at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duly noted. This is a very small data frame, so I'll leave it as is, but in the refactor for v3.5 where all scripts are evaluated, I will take this point into consideration.

BasMonkey
BasMonkey previously approved these changes Feb 20, 2026
Copy link
Contributor

@BasMonkey BasMonkey left a comment

Choose a reason for hiding this comment

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

LGTM

@mraves2 mraves2 merged commit 1bcedbd into develop Feb 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants