search

SebKrantz / collapse

Advanced and Fast Data Transformation in R
https://sebkrantz.github.io/collapse/
Other
651 stars 34 forks source link

Explicitly set locale for some test files #603

Closed MichaelChirico closed 3 months ago

MichaelChirico commented 3 months ago

I was seeing naming mismatch errors without setting locale to C here.

SebKrantz commented 3 months ago

Thanks @MichaelChirico! Wondering what brings me the honor of you making these edits to collapse's testing facilities?

Also wondered, did CRAN force you to do something about 

Result: NOTE
  File ‘data.table/libs/data_table.so’:
    Found non-API calls to R: ‘LEVELS’, ‘NAMED’, ‘SETLENGTH’,
      ‘SET_GROWABLE_BIT’, ‘SET_S4_OBJECT’, ‘SET_TRUELENGTH’,
      ‘UNSET_S4_OBJECT’

? I have some of the same notes now.

MichaelChirico commented 3 months ago

Wondering what brings me the honor of you making these edits to collapse's testing facilities?

Finally got around to importing {collapse} as a third-party package internally. We have a "unique" testing environment different from CRAN which can dredge up these sorts of edge cases.

Also wondered, did CRAN force you to do something about

We have not submitted to CRAN yet, not sure what will be enforced. Follow along at https://github.com/Rdatatable/data.table/issues/6180 :)

SebKrantz commented 3 months ago

Ok, thanks! And cool that collapse can now be used at Google :)

SebKrantz commented 3 months ago

Probably not the way you'll go about it @MichaelChirico, but for the radixsort I implemented a custom TRUELENGTH/SET_TRUELENGTH, which avoids an ALTREP check that made the function noticeably more expensive: https://github.com/SebKrantz/collapse/blob/master/src/base_radixsort.h.

MichaelChirico commented 3 months ago

Thanks for sharing! I'd still like to strive to use the public API if possible, let's see how it goes :)