Repository navigation
jsontable(x::AbstractVector{<:JSON3.Object}) - #23
anandijain wants to merge 1 commit into
Conversation
|
I think just uncommenting it should be fine? Were you seeing issues not specifying a type paramter? |
Codecov ReportBase: 91.57% // Head: 92.55% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #23 +/- ##
==========================================
+ Coverage 91.57% 92.55% +0.97%
==========================================
Files 1 1
Lines 95 94 -1
==========================================
Hits 87 87
+ Misses 8 7 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
quinnj
left a comment
There was a problem hiding this comment.
[P2] Retain the JSON-array validation fallback (src/JSONTables.jl:34).
The bounded object-vector method is the right direction, but removing the generic JSON3.Array method changes invalid-input behavior. On this exact head (c6fac42d4a2b5d177407297cc198f612fa237d7e), four public checks fail: jsontable(JSON3.read("[]")) now succeeds, while parsed [1,2], [{},1], and [null] throw MethodError instead of the existing ArgumentError. The remaining generic wrapper attempts to parse those already parsed arrays again.
Keep the validation fallback and add a more specific JSON3.Array{JSON3.Object} bridge to the bounded vector method. This avoids the dispatch ambiguity while retaining the invalid-array contract. I implemented that approach, with initial-empty-row handling and public regressions, in #26; its six current-head checks are green and it awaits independent approval.
AI disclosure: This work was prepared with assistance from OpenAI Codex.
@quinnj
This PR making me realize I'm bad at type parameters, I think we still want the
jsontable(x::JSON3.Array)dispatch that I commented out.Should I make it
jsontable(x::JSON3.Array{Any})or something?Thanks