Skip to content

Conversation

@quinnj
Copy link
Member

@quinnj quinnj commented Jul 9, 2020

Fixes #680. Before custom types, the typemap keyword argument was
really only about mapping between the standard, supported types. With
custom types, we still only support certain type mappings (Int to Float,
Any type to String), but we also want to support type mappings like
Int64 => Int32. This PR readjusts how typemap works when detecting
column types to account for the possiblity of custom Integer or
AbstractFloat type mappints for Int64 & Float64, and moving directly to
String if that's specified.

Fixes #680. Before custom types, the `typemap` keyword argument was
really only about mapping between the standard, supported types. With
custom types, we still only support certain type mappings (Int to Float,
Any type to String), but we also want to support type mappings like
`Int64 => Int32`. This PR readjusts how typemap works when detecting
column types to account for the possiblity of custom Integer or
AbstractFloat type mappints for Int64 & Float64, and moving directly to
String if that's specified.
@codecov
Copy link

codecov bot commented Jul 9, 2020

Codecov Report

Merging #684 into master will decrease coverage by 0.26%.
The diff coverage is 84.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #684      +/-   ##
==========================================
- Coverage   84.42%   84.16%   -0.27%     
==========================================
  Files          10       10              
  Lines        1779     1800      +21     
==========================================
+ Hits         1502     1515      +13     
- Misses        277      285       +8     
Impacted Files Coverage Δ
src/file.jl 95.01% <83.67%> (-1.29%) ⬇️
src/chunks.jl 100.00% <100.00%> (ø)
src/header.jl 94.96% <100.00%> (ø)
src/rows.jl 93.16% <100.00%> (ø)

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 d4ce5b6...df2541b. Read the comment docs.

@quinnj quinnj merged commit 0238a22 into master Jul 9, 2020
@quinnj quinnj deleted the jq/680 branch July 9, 2020 07:08
@kragol
Copy link
Contributor

kragol commented Jul 9, 2020

Wow that was fast. Thanks a lot for the quick fix and the detailed explanation.

quinnj added a commit that referenced this pull request Jan 12, 2026
Fix typemap for our new custom types world
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.

weird typemap behavior (bug?)

3 participants