Skip to content

fix: resolve gDRstyle linting violations#121

Open
bczech wants to merge 3 commits into
mainfrom
GDR-1014
Open

fix: resolve gDRstyle linting violations#121
bczech wants to merge 3 commits into
mainfrom
GDR-1014

Conversation

@bczech
Copy link
Copy Markdown
Contributor

@bczech bczech commented May 18, 2026

Description

What changed?

Related JIRA issue: GDR-1014

  • Fix trailing whitespace and blank lines
  • Replace nrow/ncol with NROW/NCOL (undesirable_function_linter)
  • Fix paste_linter violations (paste0+collapse, toString, strrep, file.path)
  • Fix seq_linter violations (seq_along, seq_len)
  • Refactor functions exceeding cyclocomp limit of 25 (extract helper functions)
  • Fix test linters (yoda_test, expect_true_false)
  • Remove undesirable operators (|> pipes)

Why was it changed?

Updated gDRstyle linting rules require compliance across all gDR packages.

Checklist for sustainable code base

  • I added tests for any code changed/added
  • I added documentation for any code changed/added
  • I made sure naming of any new functions is self-explanatory and consistent

Logistic checklist

  • Package version bumped
  • Changelog updated

bczech added 2 commits May 14, 2026 17:40
Address lint violations from updated gDRstyle rules:
- Fix trailing whitespace and blank lines
- Replace nrow/ncol with NROW/NCOL
- Fix paste_linter (paste0+collapse, toString, strrep, file.path)
- Fix seq_linter (seq_along, seq_len)
- Refactor cyclocomp_linter violations (extract helper functions)
- Fix test linters (yoda, expect_true_false)
- Remove undesirable operators (|> pipes)
@bczech bczech requested a review from a team as a code owner May 18, 2026 08:17
@bczech bczech requested review from darsoo and gladkia and removed request for a team May 18, 2026 08:17
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the D300 and EnVision data import logic by extracting complex parsing and matrix-filling routines into dedicated internal helper functions, enhancing modularity and readability. Robustness is improved through the use of NCOL() and NROW() for dimension checks and safer XML dimension extraction. Additionally, the PR updates test dependencies from qs2 to qs and performs extensive whitespace cleanup across the codebase. Review feedback suggests further optimizing matrix initialization in the new helper functions by avoiding unnecessary rep() calls.

Comment thread R/D300.R
Comment on lines +168 to +169
conc_mat <- matrix(rep("", nwells), nrow = nrow, ncol = ncol)
drug_mat <- matrix(rep("", nwells), nrow = nrow, ncol = ncol)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For slightly better performance and readability, you can initialize the matrices directly without using rep().

  conc_mat <- matrix("", nrow = nrow, ncol = ncol)
  drug_mat <- matrix("", nrow = nrow, ncol = ncol)

Comment thread R/D300.R
Comment on lines +193 to +194
conc_mat <- matrix(rep("", nwells), nrow = nrow, ncol = ncol)
drug_mat <- matrix(rep("", nwells), nrow = nrow, ncol = ncol)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For slightly better performance and readability, you can initialize the matrices directly without using rep().

  conc_mat <- matrix("", nrow = nrow, ncol = ncol)
  drug_mat <- matrix("", nrow = nrow, ncol = ncol)

Copy link
Copy Markdown
Contributor

@gladkia gladkia left a comment

Choose a reason for hiding this comment

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

LGTM

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