drop json3#571
Conversation
There was a problem hiding this comment.
Pull request overview
Updates InfrastructureSystems’ JSON serialization/deserialization stack to drop JSON3 (and StructTypes) in favor of JSON.jl, aligning with upcoming OpenAPI model usage and the JSON3→JSON migration guidance.
Changes:
- Replaced JSON3 read/write/pretty calls across src and tests with JSON.parse / JSON.json.
- Removed JSON3/StructTypes-specific enum and struct serialization hooks and updated corresponding deserialize paths.
- Updated package dependencies (Project.toml + test/Project.toml) and test imports to use JSON.jl.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_time_series.jl | Switches metadata JSON encoding used in migration setup helpers. |
| test/test_serialization.jl | Updates serialization round-trip tests to JSON.jl parse/encode APIs. |
| test/test_recorder.jl | Updates event log parsing to JSON.jl. |
| test/test_generate_structs.jl | Updates generated-struct tests to JSON.jl parsing. |
| test/test_function_data.jl | Updates JSON “round-trip to Dict” test logic to JSON.jl. |
| test/Project.toml | Replaces JSON3 test dependency with JSON. |
| test/InfrastructureSystemsTests.jl | Replaces import JSON3 with import JSON. |
| src/validation.jl | Switches descriptor JSON parsing to JSON.jl. |
| src/utils/utils.jl | Replaces JSON3 usage; updates @scoped_enum JSON-related behavior; updates file-hash JSON write. |
| src/utils/test.jl | Switches test helper JSON parsing to JSON.jl. |
| src/utils/recorder_events.jl | Switches recorder event JSON encode/decode to JSON.jl. |
| src/utils/generate_structs.jl | Switches descriptor JSON parsing to JSON.jl. |
| src/utils/generate_struct_files.jl | Switches descriptor JSON parsing/writing and removes JSON3/StructTypes hooks for struct serialization. |
| src/time_series_parser.jl | Switches time-series metadata JSON parsing to JSON.jl. |
| src/time_series_metadata_store.jl | Switches metadata/features/scaling-factor JSON parse/write to JSON.jl. |
| src/serialization.jl | Switches core to_json/from_json and ext validation to JSON.jl. |
| src/abstract_time_series.jl | Switches feature-string JSON encoding to JSON.jl. |
| src/InfrastructureSystems.jl | Replaces JSON3 import with JSON; removes StructTypes import. |
| Project.toml | Replaces JSON3/StructTypes deps with JSON and updates compat accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| io = IOBuffer() | ||
| JSON3.pretty(io, serialize(obj), JSON3.AlignmentContext(; indent = indent)) | ||
| JSON.json(io, serialize(obj); pretty = indent) | ||
| return take!(io) |
There was a problem hiding this comment.
to_json(obj; pretty=true) is documented as returning a JSON string, but this branch returns take!(io) (a Vector{UInt8}) while the non-pretty branch returns a String. This inconsistent return type can surprise callers and makes downstream parsing more brittle. Consider returning a String consistently (e.g., wrap the buffer with String(...)) or updating the docstring/return contract if bytes are intentional.
| return take!(io) | |
| return String(take!(io)) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## IS4 #571 +/- ##
==========================================
- Coverage 82.04% 81.86% -0.18%
==========================================
Files 74 74
Lines 6238 6304 +66
==========================================
+ Hits 5118 5161 +43
- Misses 1120 1143 +23
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
luke-kiernan
left a comment
luke-kiernan
left a comment
There was a problem hiding this comment.
A little bit outside what I typically deal with, but I see no issues.
To support the upcoming usage of OpenAPI models. I am updating to JSON.jl from JSON3 according to the migration guidelines. This PR needs to be followed with a PSY one