Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Generalized sort to accept indices other than i32. #220

Merged
merged 1 commit into from
Jul 24, 2021
Merged

Conversation

jorgecarleitao
Copy link
Owner

@jorgecarleitao jorgecarleitao commented Jul 24, 2021

This PR generalizes sort to accept indices other than i32 (i.e. u32, u64, i32, i64). Some types may be more advantageous than others depending on the use-case, and since Rust enables this quite easily, we may as well allow it.

Note that this does increase the overall compilation of dependents as sort is now a generic over I and is thus compiled on the dependent crate.

This is backward incompatible. To recover the original behavior, add ::<i32> to functions that return indices in compute::sort. Change to e.g. u64 to return a UInt64Array instead.

This has no impact in performance.

@jorgecarleitao jorgecarleitao added backwards-incompatible enhancement An improvement to an existing feature labels Jul 24, 2021
@codecov
Copy link

codecov bot commented Jul 24, 2021

Codecov Report

Merging #220 (f7d3341) into main (a6e8b69) will increase coverage by 0.00%.
The diff coverage is 55.83%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #220   +/-   ##
=======================================
  Coverage   76.93%   76.94%           
=======================================
  Files         229      229           
  Lines       19533    19536    +3     
=======================================
+ Hits        15028    15031    +3     
  Misses       4505     4505           
Impacted Files Coverage Δ
src/compute/sort/mod.rs 53.55% <20.63%> (+0.16%) ⬆️
src/array/specification.rs 72.34% <75.00%> (+2.57%) ⬆️
src/compute/sort/common.rs 93.42% <96.77%> (+1.01%) ⬆️
src/compute/sort/boolean.rs 94.44% <100.00%> (ø)
src/compute/sort/lex_sort.rs 81.98% <100.00%> (-0.17%) ⬇️
src/compute/sort/primitive/indices.rs 100.00% <100.00%> (ø)
src/compute/sort/utf8.rs 100.00% <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 a6e8b69...f7d3341. Read the comment docs.

@jorgecarleitao jorgecarleitao merged commit eaa9be9 into main Jul 24, 2021
@jorgecarleitao jorgecarleitao deleted the sort_indice branch July 24, 2021 20:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backwards-incompatible enhancement An improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant