Skip to content

Conversation

lbilli
Copy link

@lbilli lbilli commented Jul 1, 2019

To address the loss of eltype information in numeric arrays, I tried to implement the following logic:

  • if at least a float is encountered, then eltype is Float64, otherwise it's Int64
  • if a null is encounterd, then eltype is the union of the previous item and Nothing.

The only possible eltypes for numerical arrays should be Int64, Float64, Union{Int64,Nothing} and Union{Float64,Nothing}.

Any should appear only if something that is not a Int, Float or null is encountered.

@codecov
Copy link

codecov bot commented Jul 1, 2019

Codecov Report

Merging #13 into master will decrease coverage by 0.2%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
- Coverage   96.47%   96.27%   -0.21%     
==========================================
  Files           7        7              
  Lines         993      993              
==========================================
- Hits          958      956       -2     
- Misses         35       37       +2
Impacted Files Coverage Δ
src/JSON3.jl 96.61% <100%> (ø) ⬆️
src/utils.jl 92.7% <75%> (-2.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3841adf...dd4620b. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jul 1, 2019

Codecov Report

Merging #13 into master will decrease coverage by 0.2%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
- Coverage   96.47%   96.27%   -0.21%     
==========================================
  Files           7        7              
  Lines         993      993              
==========================================
- Hits          958      956       -2     
- Misses         35       37       +2
Impacted Files Coverage Δ
src/JSON3.jl 96.61% <100%> (ø) ⬆️
src/utils.jl 92.7% <75%> (-2.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3841adf...dd4620b. Read the comment docs.

@@ -98,7 +98,7 @@ function Base.iterate(arr::Array{T}, (i, tapeidx)=(1, 3)) where {T}
i > length(arr) && return nothing
tape = gettape(arr)
@inbounds t = tape[tapeidx]
val = getvalue(T, getbuf(arr), tape, tapeidx, t)
val = getvalue(Any, getbuf(arr), tape, tapeidx, t)
Copy link
Owner

Choose a reason for hiding this comment

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

this is bad; this is how we can get fast, type-stable code when accessing elements of JSON3.Array{T}

@@ -561,6 +561,10 @@ txt = """
# https://github.com/quinnj/JSON3.jl/issues/8
@test eltype(JSON3.read("[1.2, 2.0]")) === Float64
@test eltype(JSON3.read("[1.2, 2.0, 3.3]")) === Float64
@test eltype(JSON3.read("[1, 2]")) == Int64
@test eltype(JSON3.read("[1, 2.3]")) == Float64
Copy link
Owner

Choose a reason for hiding this comment

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

This case is problematic; the problem is that parsing already parsed 1 as an Int64, so we can't have the final eltype be Float64 or it will reinterpret the first element wrong (it will reinterpret the bits as Float64 when the bits are encoded as Int64).

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.

2 participants