Skip to content

Conversation

@jangorecki
Copy link
Member

@jangorecki jangorecki commented Aug 13, 2019

closes #3913

Internally used in nafill function coerceFill has been heavily modified, and renamed to coerceClass. Main purpose of the function is to handle coercion between types/classes with care on int64.
Comparing to previous coerceFill version, new coerceClass:

  • handle coerce of vector, not only scalar
  • handle coercion from any class to any class
  • int64 coercion is taken care for integer and double only
  • coercion between int / double / int64 is handled in the function, otherwise R's coerceVector is used - as a result, consider char to int64 (coerceClass("1", list(integer(0), bit64::as.integer64(0)))). This is going to be addressed in this PR, placeholders are in place. Also there might be some missing warnings like (NA introduced by coercion, Int overflow), which are currently not checked/issued in our int / double / int64 handling.

utils.c tests moved into own test script


motivation:

  • extend nafill for types other than integer, numeric and int64
  • to be re-used as efficient way to handle conversions like
data.table(a=1:2, b=bit64::as.integer64(1:2))[2L, c("a","b") := NA][]
#       a                   b
#   <int>               <i64>
#1:     1                   1
#2:    NA 9218868437227407266
data.table(a=1:2, b=bit64::as.integer64(1:2))[2L, c("a","b") := coerceClass(NA, .SD)][]
#   <int> <i64>
#1:     1     1
#2:    NA  <NA>

in assign, rbindlist and potentially other places where we need to take care of int64 NAs handling or other classes like #3654


As of current moment I don't understand coverage issue, it reports few closing brackets } that are not being covered. Maybe a matter of waiting for update.

@jangorecki jangorecki added the WIP label Aug 13, 2019
@codecov
Copy link

codecov bot commented Aug 13, 2019

Codecov Report

Merging #3765 into master will decrease coverage by 0.02%.
The diff coverage is 99.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3765      +/-   ##
==========================================
- Coverage   99.41%   99.38%   -0.03%     
==========================================
  Files          71       71              
  Lines       13155    13272     +117     
==========================================
+ Hits        13078    13191     +113     
- Misses         77       81       +4
Impacted Files Coverage Δ
src/init.c 100% <ø> (ø) ⬆️
R/wrappers.R 100% <100%> (ø) ⬆️
src/nafill.c 100% <100%> (ø) ⬆️
src/utils.c 98.1% <99.21%> (-1.9%) ⬇️

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 a2e7f5e...2ce3119. Read the comment docs.

@jangorecki
Copy link
Member Author

This PR should also take care of #3913

@jangorecki jangorecki mentioned this pull request May 14, 2020
14 tasks
@jangorecki
Copy link
Member Author

superseded by #4491

@jangorecki jangorecki closed this May 26, 2020
@MichaelChirico MichaelChirico deleted the cutils2 branch July 8, 2025 17:30
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.

class-aware C coercion function, no-alloc coerce where possible

2 participants