Skip to content

Dev json hot fix#1070

Merged
kellijohnson-NOAA merged 3 commits into
mainfrom
dev-json-hot-fix
Dec 17, 2025
Merged

Dev json hot fix#1070
kellijohnson-NOAA merged 3 commits into
mainfrom
dev-json-hot-fix

Conversation

@msupernaw

Copy link
Copy Markdown
Contributor

What is the feature?

  • Fixes JSON format issues associated with nan and missing values.

How have you implemented the solution?

  • Created a sanitization function and added appropriate terminating character

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

  • no

@kellijohnson-NOAA

Copy link
Copy Markdown
Contributor

Thanks for this fix @msupernaw but you opened a PR to main using a feature branch off of dev. This seems like it is a true hot fix and thus your branch needs to be rebased to main and force pushed.

@msupernaw msupernaw changed the base branch from main to dev December 12, 2025 19:36
@msupernaw

Copy link
Copy Markdown
Contributor Author

@kellijohnson-NOAA I meant to create the PR to dev.

@codecov

codecov Bot commented Dec 12, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.96%. Comparing base (e0be2a1) to head (0d7f735).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...nterface/rcpp/rcpp_objects/rcpp_interface_base.hpp 71.42% 2 Missing ⚠️
...nclude/interface/rcpp/rcpp_objects/rcpp_models.hpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1070      +/-   ##
==========================================
- Coverage   85.97%   85.96%   -0.01%     
==========================================
  Files          81       81              
  Lines        7884     7887       +3     
  Branches      515      517       +2     
==========================================
+ Hits         6778     6780       +2     
- Misses       1098     1099       +1     
  Partials        8        8              

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

@kellijohnson-NOAA

Copy link
Copy Markdown
Contributor

But the PR should be to main because it fixes version 0.7.0. So, the DESCRIPTION file should also be edited to be 0.7.1 because it is a hot fix and not a new version. If it goes into dev first then it will be part of version 0.8.0. Also, the spell check file needs to be updated.

@msupernaw msupernaw changed the base branch from dev to main December 15, 2025 14:31
@kellijohnson-NOAA

Copy link
Copy Markdown
Contributor

This PR needs to be rebased to main because it was originally based off of a feature branch off of dev.

git fetch -a
git checkout dev-json-hot-fix
git rebase origin/main

@msupernaw

Copy link
Copy Markdown
Contributor Author

@kellijohnson-NOAA rebased to main yesterday. I just verified the SHAs are the same, let me know if there are issues.

@kellijohnson-NOAA kellijohnson-NOAA linked an issue Dec 16, 2025 that may be closed by this pull request
@kellijohnson-NOAA

Copy link
Copy Markdown
Contributor

Thanks @msupernaw I didn't realize that because you made a feature branch off of dev and dev has the exact same history as main that it won't allow the removal of those two commits (see below) with a straight rebase. You need to do an interactive rebase and remove those commits. Let me know if you need help doing that. Gemini can also provide directions
image

Gemini instructions
Step 1: Start the Interactive Rebase
Make sure you are on your feature branch and run the rebase command against main:

git checkout your-feature-branch
git rebase -i main

Step 2: Drop the Unwanted Commits
A text editor (usually Vim, Nano, or VS Code) will open showing a list of commits that are on your branch but not on main. It will look something like this:

pick a1b2c3d Fix layout bug (Your feature commit)
pick e5f6g7h Update dependencies (The unwanted dev commit 1)
pick i8j9k0l Add logging (The unwanted dev commit 2)
pick m1n2o3p Update documentation (Your feature commit)

Locate the two commits from dev that you want to remove. Change the word pick to drop (or just d) next to those commits Leave your actual feature commits as pick.
It should now look like this:

pick a1b2c3d Fix layout bug
drop e5f6g7h Update dependencies
drop i8j9k0l Add logging
pick m1n2o3p Update documentation

Step 3: Save and Exit
If in Vim: Press Esc, type :wq, and hit Enter.
If in Nano: Press Ctrl+O, Enter to save, then Ctrl+X to exit.
If in VS Code: Just save the file and close the tab.

Git will now rewrite your branch history, removing the "dropped" commits and keeping only your feature work.

Step 4: Force Push
Because you have rewritten the history of the branch, you cannot do a normal push. You must force push to update the remote server:

git push --force-with-lease origin your-feature-branch

(Note: --force-with-lease is safer than --force because it checks that no one else has pushed code to that branch in the meantime.)

@kellijohnson-NOAA kellijohnson-NOAA force-pushed the dev-json-hot-fix branch 2 times, most recently from 0a1c53d to 8bc01bf Compare December 16, 2025 23:20
msupernaw and others added 3 commits December 16, 2025 15:22
Thanks to @MOshima-PIFSC for finding the bug and @msupernaw for fixing it.
Adds a sanitization function to fix the output formatting from model
estimation in rcpp_models.hpp rcpp_interface_base.hpp

Close #1063
@kellijohnson-NOAA kellijohnson-NOAA linked an issue Dec 17, 2025 that may be closed by this pull request
@kellijohnson-NOAA kellijohnson-NOAA merged commit c9e8c0c into main Dec 17, 2025
17 checks passed
@kellijohnson-NOAA kellijohnson-NOAA deleted the dev-json-hot-fix branch December 17, 2025 16:54
@e-perl-NOAA e-perl-NOAA mentioned this pull request May 14, 2026
5 tasks
kellijohnson-NOAA pushed a commit that referenced this pull request May 20, 2026
kellijohnson-NOAA pushed a commit that referenced this pull request May 20, 2026
kellijohnson-NOAA pushed a commit that referenced this pull request May 20, 2026
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:] SB is blank in print of fit [Bug:] invalid string in json text

2 participants