Skip to content

support user-defined mapping for Inf and NaN via keyword arg #294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hhaensel wants to merge 8 commits into
base: main
Choose a base branch
from
Open

support user-defined mapping for Inf and NaN via keyword arg #294

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d68937d
support user-defined mapping for Inf and NaN via keyword arg
hhaensel Nov 17, 2024
7d887c6
Merge branch 'quinnj:main' into hh-infinity2
hhaensel Jan 2, 2025
1ce1b28
allow_inf: fix lower performance for default_mapping, add `underscore…
Jan 15, 2025
f656813
remove custom inf_mappings and add kw inf_mapping to docstring
hhaensel Jan 15, 2025
62f8efb
modify docstring for inf_mapping
Jan 16, 2025
0dc0277
revise docstring of write, add brackets for clarity
hhaensel Jan 29, 2025
4fc6b43
support `read()` with inf_mapping
Jan 31, 2025
7f08318
include quotes in test when reading inf and nan strings
Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions src/read.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@ end

const FLOAT_INT_BOUND = 2.0^53

function read!(buf, pos, len, b, tape, tapeidx, ::Type{Any}, checkint=true; allow_inf::Bool=false)
function read!(buf, pos, len, b, tape, tapeidx, ::Type{Any}, checkint=true; inf_mapping::Union{Function,Nothing}=nothing, allow_inf::Bool=(inf_mapping !== nothing))
if b == UInt8('{')
return read!(buf, pos, len, b, tape, tapeidx, Object, checkint; allow_inf=allow_inf)
return read!(buf, pos, len, b, tape, tapeidx, Object, checkint; allow_inf=allow_inf, inf_mapping=inf_mapping)
elseif b == UInt8('[')
return read!(buf, pos, len, b, tape, tapeidx, Array, checkint; allow_inf=allow_inf)
return read!(buf, pos, len, b, tape, tapeidx, Array, checkint; allow_inf=allow_inf, inf_mapping=inf_mapping)
elseif b == UInt8('"')
return read!(buf, pos, len, b, tape, tapeidx, String)
return read!(buf, pos, len, b, tape, tapeidx, String; inf_mapping=inf_mapping)
elseif b == UInt8('n')
return read!(buf, pos, len, b, tape, tapeidx, Nothing)
elseif b == UInt8('t')
Expand Down Expand Up @@ -148,7 +148,7 @@ function read!(buf, pos, len, b, tape, tapeidx, ::Type{Any}, checkint=true; allo
invalid(InvalidChar, buf, pos, Any)
end

function read!(buf, pos, len, b, tape, tapeidx, ::Type{String})
function read!(buf, pos, len, b, tape, tapeidx, ::Type{String}; inf_mapping::Union{Function,Nothing}=nothing)
pos += 1
@eof
strpos = pos
Expand All @@ -171,6 +171,23 @@ function read!(buf, pos, len, b, tape, tapeidx, ::Type{String})
b = getbyte(buf, pos)
end
@check
if inf_mapping !== nothing
val = view(buf, strpos:pos-1)
float = if val == codeunits(inf_mapping(Inf))[2:end-1]
Inf
elseif val == codeunits(inf_mapping(-Inf))[2:end-1]
-Inf
elseif val == codeunits(inf_mapping(NaN))[2:end-1]
Copy link
Contributor

@LilithHafner LilithHafner Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This usage makes me think that inf_mapping should be a Tuple or NamedTuple rather than a function.

Copy link
Author

@hhaensel hhaensel Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so, too. But the function version was much faster.

Copy link
Author

@hhaensel hhaensel Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried arrays, tuples and functions, at least concerning writing. I didn't check read performance.

Copy link
Author

@hhaensel hhaensel Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also checked the RawType approach but I couldn't find out how to change the type to Float. The current approach looks more natural to me and has less code.
It is somewhat of a restriction that I only support the case of string mappings, but I think it is very untypical that people want to cover other values than Infinity and NaN if they have a process that allows to send non-standard JSON.
EDIT: it's easy to include the quotes just by expanding the view and leaving out the [2:end-1] so my previous comment is no longer valid.

Copy link
Contributor

@LilithHafner LilithHafner Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference between function and tuple that is giving you that performance difference is methods are specialized on a function but not on a tuple's value. You could get similar performance with a tuple by lifting it to the type domain with Val. I do see the advantage in terms of runtime performance of having the serialization format of inf and nan be passed into write/read at the type level

NaN
else
0.0
end
if float != 0.0
@inbounds tape[tapeidx] = FLOAT
@inbounds tape[tapeidx+1] = Core.bitcast(UInt64, float)
return pos + 1, tapeidx + 2
end
end
@inbounds tape[tapeidx] = string(strlen)
@inbounds tape[tapeidx+1] = ifelse(escaped, ESCAPE_BIT | strpos, strpos)
return pos + 1, tapeidx + 2
Expand Down