Skip to content

fix quoted bug#89

Merged
tonywu1999 merged 1 commit intodevelfrom
fix-bug
Mar 31, 2026
Merged

fix quoted bug#89
tonywu1999 merged 1 commit intodevelfrom
fix-bug

Conversation

@tonywu1999
Copy link
Copy Markdown
Contributor

@tonywu1999 tonywu1999 commented Mar 25, 2026

Motivation and Context

Please include relevant motivation and context of the problem along with a short summary of the solution.

Changes

Please provide a detailed bullet point list of your changes.

Testing

Please describe any unit tests you added or modified to verify your changes.

Checklist Before Requesting a Review

  • I have read the MSstats contributing guidelines
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • Ran styler::style_pkg(transformers = styler::tidyverse_style(indent_by = 4))
  • Ran devtools::document()

Motivation and Context

The renderCytoscapeNetwork() function contained a critical bug where it referenced an undefined variable quoted in a conditional statement. The function signature only includes expr and env parameters, but the code attempted to check if (!quoted) before calling substitute(expr). This would cause a runtime error whenever the function is invoked in a Shiny application, as the quoted variable does not exist in the function's scope.

The fix simplifies the logic by unconditionally calling substitute(expr) before passing the expression to htmlwidgets::shinyRenderWidget() with quoted = TRUE. This is the correct pattern for Shiny render functions that need to capture the unevaluated expression from the caller.

Changes

  • R/cytoscapeNetwork.R: Modified renderCytoscapeNetwork() function to remove the undefined quoted variable reference and unconditionally call substitute(expr) on the input expression before passing it to htmlwidgets::shinyRenderWidget(). The quoted argument to shinyRenderWidget() remains TRUE, ensuring proper expression handling in the widget rendering pipeline.

Unit Tests

The existing test suite in tests/testthat/test-utils_cytoscapeNetwork.R covers the helper functions and the public cytoscapeNetwork() API, including validation of input parameters, element building, and widget creation. These tests verify the core functionality of the network visualization pipeline. However, no explicit unit tests are documented in the PR for the renderCytoscapeNetwork() function itself, which would typically be integration-tested within a Shiny application context.

Coding Guidelines

No violations of coding guidelines are evident in this change. The fix aligns with standard R/Shiny patterns for render functions and represents a straightforward bug correction.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5c0581e3-9bfe-4cae-92d3-c0901d2f4c84

📥 Commits

Reviewing files that changed from the base of the PR and between a5e9c4d and dd7f8e3.

📒 Files selected for processing (1)
  • R/cytoscapeNetwork.R

📝 Walkthrough

Walkthrough

The renderCytoscapeNetwork() function in R/cytoscapeNetwork.R is modified to unconditionally call substitute(expr) before passing the expression to htmlwidgets::shinyRenderWidget(), removing the prior conditional logic based on a quoted variable.

Changes

Cohort / File(s) Summary
Expression Substitution Logic
R/cytoscapeNetwork.R
Removed conditional substitution of expr argument; substitute(expr) is now called unconditionally before passing to shinyRenderWidget(), while the quoted parameter remains TRUE.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A condition removed with a gentle hop,
Where once we chose, now substitution won't stop—
Simplified logic, direct and pure,
One less branch to ensure,
The code hops forward, cleaner than before!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description uses the template structure but provides no substantive content—motivation, specific changes, and testing details are all missing or incomplete. Complete the description by explaining the bug, detailing the changes to renderCytoscapeNetwork's expr argument handling, and describing any tests added to verify the fix.
Title check ❓ Inconclusive The title 'fix quoted bug' is vague and generic, lacking specificity about what the bug is or which component is affected. Revise the title to be more descriptive, such as 'Fix quoted argument handling in renderCytoscapeNetwork' to clearly indicate what bug was fixed.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-bug

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 76.79%. Comparing base (a5e9c4d) to head (dd7f8e3).

Files with missing lines Patch % Lines
R/cytoscapeNetwork.R 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            devel      #89   +/-   ##
=======================================
  Coverage   76.79%   76.79%           
=======================================
  Files           8        8           
  Lines        1030     1030           
=======================================
  Hits          791      791           
  Misses        239      239           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tonywu1999 tonywu1999 merged commit a26e6fb into devel Mar 31, 2026
4 checks passed
@tonywu1999 tonywu1999 deleted the fix-bug branch March 31, 2026 21:50
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