Skip to content

v1.37.2 - fix subsetting of grouped data frame#109

Merged
oss6 merged 6 commits intomasterfrom
fix/average_xcms_grouped_msms_indiv
Feb 26, 2026
Merged

v1.37.2 - fix subsetting of grouped data frame#109
oss6 merged 6 commits intomasterfrom
fix/average_xcms_grouped_msms_indiv

Conversation

@oss6
Copy link
Copy Markdown
Member

@oss6 oss6 commented Feb 20, 2026

The grpid column in pa@grped_df is an integer factor. The comparison pa@grped_df$grpid==as.numeric(grp_idx) fails when grp_idx is 100000 because as.numeric returns type double.

pa@grped_df$grpid == 100000 returns FALSE for all items even if 100000 is present. pa@grped_df$grpid == 100000L returns TRUE for items that are 100000.

oss6 and others added 4 commits February 19, 2026 21:46
The `grpid` column in `pa@grped_df` is an integer factor.
The comparison `pa@grped_df$grpid==as.numeric(grp_idx)` fails when `grp_idx`
is 100000 because `as.numeric` returns type `double`.

`pa@grped_df$grpid == 100000` returns `FALSE` for all items even if 100000 is present.
`pa@grped_df$grpid == 100000L` returns `TRUE` for items that are 100000.
…ata package (too large for Bioconductor) (#108)

* Use zenodo reference to library spectra

* Update CI/CD

* troubleshoot vignette build issue

* Troubleshooting cache on github workflows

* Troubleshooting cache for github workflows

* more troubleshooting GH workflow cache
@oss6 oss6 changed the title Fix subsetting of grouped data frame v1.37.2 - fix subsetting of grouped data frame Feb 20, 2026
@oss6 oss6 requested a review from Tomnl February 20, 2026 09:17
Copy link
Copy Markdown
Member

@Tomnl Tomnl left a comment

Choose a reason for hiding this comment

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

Good spot @oss6.

I’ve not previously worked with a dataset containing ≥100,000 grouped features, so it makes sense that this didn’t surface before.

The updated handling of grp_idx looks much safer. Hopefully, if the same unit tests are passing after the fix, and given that the issue caused the analysis to fail in the scenario you identified, this suggests the issue is likely confined to that specific case.

Please go ahead and squash and merge once all CI checks are green.

@oss6 oss6 merged commit 808690c into master Feb 26, 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