Skip to content

Fix new Parameter crash in R#1364

Merged
kellijohnson-NOAA merged 4 commits into
devfrom
dev-fix-new-parameter-crash
Mar 26, 2026
Merged

Fix new Parameter crash in R#1364
kellijohnson-NOAA merged 4 commits into
devfrom
dev-fix-new-parameter-crash

Conversation

@Andrea-Havron-NOAA

@Andrea-Havron-NOAA Andrea-Havron-NOAA commented Mar 26, 2026

Copy link
Copy Markdown
Collaborator

What is the feature?

How have you implemented the solution?

*- change the exposed Parameter constructor to SEXP type

  • add a validation function in the rcpp interface
  • update the error message in the rcpp-fims test

Does the PR impact any other area of the project, maybe another repo?

  • fixes update-pkgdown GHA

Instructions for code reviewer

👋Hello reviewer👋, thank you for taking the time to review this PR!

  • Please use this checklist during your review, checking off items that you have verified are complete but feel free to skip over items that are not relevant!
  • See the GitHub documentation for how to comment on a PR to indicate where you have questions or changes are needed before approving the PR.
  • Please use standard conventional messages for both commit messages and comments
  • PR reviews are a great way to learn so feel free to share your tips and tricks. However, when suggesting changes to the PR that are optional please include nit: (for nitpicking) as the comment type. For example, nit: I prefer using a data.frame() instead of a matrix because ...
  • Engage with the developer. Make it clear when the PR is approved by selecting the approved status, and potentially commenting on the PR with something like This PR is now ready to be merged.

Checklist

  • The PR requests the appropriate base branch (dev for features and main for hot fixes)
  • The code is well-designed
  • The code is designed well for both users and developers
  • Code coverage remains high- [ ] Comments are clear, useful, and explain why instead of what
  • Code is appropriately documented (doxygen and roxygen)

- change the exposed Parameter constructor to SEXP type
- add a validation function in the rcpp interface
- update the error message in the rcpp-fims test
@codecov

codecov Bot commented Mar 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.60%. Comparing base (befbbc4) to head (80047bd).
⚠️ Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
...nterface/rcpp/rcpp_objects/rcpp_interface_base.hpp 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1364      +/-   ##
==========================================
- Coverage   89.61%   89.60%   -0.02%     
==========================================
  Files          82       82              
  Lines        8352     8360       +8     
  Branches      580      585       +5     
==========================================
+ Hits         7485     7491       +6     
- Misses        840      844       +4     
+ Partials       27       25       -2     

☔ 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.

Comment thread inst/include/interface/rcpp/rcpp_objects/rcpp_interface_base.hpp Outdated
kellijohnson-NOAA and others added 3 commits March 25, 2026 20:01
Removes return statement in doxygen documentation for the check on the value of a Parameter.
@kellijohnson-NOAA kellijohnson-NOAA merged commit 20850d5 into dev Mar 26, 2026
18 of 19 checks passed
@kellijohnson-NOAA kellijohnson-NOAA deleted the dev-fix-new-parameter-crash branch March 26, 2026 09:24
@kellijohnson-NOAA kellijohnson-NOAA linked an issue Apr 2, 2026 that may be closed by this pull request
Rohith-1717 pushed a commit to Rohith-1717/FIMS that referenced this pull request Apr 12, 2026
- change the exposed Parameter constructor to SEXP type
- add a validation function in the rcpp interface
- update the error message in the rcpp-fims test
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.

[Bug:] call to methods::new(Parameter, "a") crashes R

2 participants