Migrate from RJSONIO to jsonlite (#193) #291
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add comprehensive validation tests for jsonlite compatibility before migrating from RJSONIO. These tests verify jsonlite with auto_unbox=TRUE can handle all animint2 data structures. Tests validate: - Single value encoding without array wrapping - Vector arrays preservation - Nested geom structures (plot.json format) - Layout data with boolean arrays - Axis, grid, selector, and panel data - Complete export.data round-trip - Valid JSON output for JavaScript parsing - Numeric precision preservation All 44 validation tests pass, confirming jsonlite is ready for migration. Actual code migration will follow in next commit. Also adds jsonlite to Suggests in DESCRIPTION for testing. Part of #193 - RJSONIO deprecation migration
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #291 +/- ##
==========================================
+ Coverage 73.06% 77.68% +4.62%
==========================================
Files 164 164
Lines 8758 8758
Branches 0 554 +554
==========================================
+ Hits 6399 6804 +405
+ Misses 2359 1954 -405
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Replace RJSONIO with jsonlite in DESCRIPTION, NAMESPACE, and all code. Use auto_unbox=TRUE and force=TRUE for toJSON, simplifyVector=FALSE for fromJSON. All tests pass. Closes #193
ANAMASGARD
force-pushed
the
migrate-rjsonio-to-jsonlite
branch
from
02a11ef to
e2b41bd
Compare
Collaborator
|
No obvious timing issues in HEAD=migrate-rjsonio-to-jsonlite Generated via commit bbcc1e7 Download link for the artifact containing the test results: ↓ atime-results.zip
|
also converts named numeric and integer vectors to JSON objects. This was causing 349 test failures in JS_coverage and R_coverage.
Without this parameter, jsonlite outputs {} instead of null for NULL values.
This was causing JavaScript errors when checking for null values in plot.json
fields like span.rowspan and span.colspan, resulting in 350 test failures
in JS_coverage and R_coverage.
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
RJSONIO is no longer maintained on CRAN (issue #193). We need to migrate to jsonlite, which is the modern standard used by Shiny, plotly, and other major R packages.
FIXES:#193
Solution
Commit 1: Validation Tests (Current)
auto_unbox=TRUEproduces correct JSON formatCommit 2: Migration (Coming Next)
auto_unbox=TRUE,dataframe="rows",null="null"Testing