Skip to content

Also it should be converted to a series of ":=" I think #210

@Rudhik1904

Description

@Rudhik1904

Also it should be converted to a series of ":=" I think

Originally posted by @mstaniak in #208 (comment)

I looked into this but think the current input[, list(...)] is actually the right call here, for two reasons:
  1. In-place := on input would corrupt the caller. .selectHighQualityFeatures is called from MSstatsSelectFeatures, which reuses the same input immediately afterwards:

features_quality = .selectHighQualityFeatures(input, min_feature_count)
input = merge(input, features_quality,
by.x = c("LABEL", "PROTEIN", "FEATURE", "originalRUN"),
by.y = c("label", "protein", "feature", "run"))
Since data.table :=/setnames modify by reference across function boundaries, renaming the columns to lowercase (or adding log2inty/is_obs) inside the function would mutate the caller's table and break that merge (its by.x keys would no longer exist). The input[, list(...)] form avoids this by returning a new table and only rebinding the local name.

  1. It's already a minimal projection, not a full-table copy. list(...) materializes only the 7 columns the feature-selection step needs — it's not the kind of whole-table duplication this PR is targeting.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions