Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #381 +/- ##
==========================================
+ Coverage 90.10% 90.20% +0.10%
==========================================
Files 7 7
Lines 1334 1348 +14
==========================================
+ Hits 1202 1216 +14
Misses 132 132 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/JSON.jl
Outdated
| """ | ||
| function writefile end | ||
|
|
||
| function writefile(::Type{Vector{UInt8}}, x; pretty::Union{Integer,Bool}=false, kw...) |
There was a problem hiding this comment.
I'm not following why we want this or the 2nd definitions? writefile that doesn't write to a file and just returns a byte vector?
There was a problem hiding this comment.
I want to avoid extra conversions between String and Vector{UInt8} for use with Zarr.jl and SmallZarrGroups.jl
I'm also trying to match the style of the writefile function in Parquet2.jl https://expandingman.gitlab.io/Parquet2.jl/#Writing-Data
There was a problem hiding this comment.
Yes, looking at this again, writefile isn't a good name for the function, so I changed it to write_json.
There was a problem hiding this comment.
Here is a benchmark on the cost of doing the extra String to Vector{UInt8} conversion:
julia> using Chairmarks, JSON, Random
julia> @be randstring(10000) Vector{UInt8}(JSON.json(_)) seconds=2
Benchmark: 52143 samples with 4 evaluations
min 4.807 μs (7 allocs: 19.852 KiB)
median 4.874 μs (7 allocs: 19.852 KiB)
mean 5.566 μs (7 allocs: 19.856 KiB, 0.82% gc time)
max 15.658 ms (7 allocs: 19.883 KiB, 99.92% gc time)
julia> @be randstring(10000) JSON.write_json(Vector{UInt8}, _) seconds=2
Benchmark: 50390 samples with 7 evaluations
min 2.971 μs (4 allocs: 9.914 KiB)
median 3.006 μs (4 allocs: 9.914 KiB)
mean 3.394 μs (4 allocs: 9.919 KiB, 0.82% gc time)
max 8.692 ms (4 allocs: 9.945 KiB, 99.92% gc time)writefile to improve compatibilitywrite_json to improve compatibility
This PR adds a
JSON.write_jsonfunction with the same functionality and keyword arguments as the newJSON.jsonfunction. This function is provided for backward compatibility when upgrading fromolder versions of JSON.jl where the
jsonfunction signature differed.The idea is that
JSON.write_jsonwill be backported to v0.21.5EDIT: Changed the name from
writefiletowrite_json