Skip to content

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Sep 6, 2022

Also adds a generic implementation of subset for column-oriented tables
and row-oriented tables that are AbstractVector. Adds implementation and
tests for DictRowTable/DictColumnTable.

Also adds a generic implementation of subset for column-oriented tables
and row-oriented tables that are AbstractVector. Adds implementation and
tests for DictRowTable/DictColumnTable.
@quinnj quinnj mentioned this pull request Sep 6, 2022
@quinnj quinnj requested review from bkamins and nalimilan September 6, 2022 16:37
@codecov
Copy link

codecov bot commented Sep 6, 2022

Codecov Report

Merging #292 (94e117b) into main (9c92441) will increase coverage by 0.13%.
The diff coverage is 96.42%.

@@            Coverage Diff             @@
##             main     #292      +/-   ##
==========================================
+ Coverage   94.94%   95.08%   +0.13%     
==========================================
  Files           7        7              
  Lines         673      692      +19     
==========================================
+ Hits          639      658      +19     
  Misses         34       34              
Impacted Files Coverage Δ
src/Tables.jl 88.62% <94.11%> (+0.62%) ⬆️
src/dicts.jl 95.57% <100.00%> (+1.18%) ⬆️
src/namedtuples.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

return ColumnsRow(cols, inds)
else
ret = view === true ? _map(c -> Base.view(c, inds), cols) : _map(c -> c[inds], cols)
return DictColumnTable(schema(cols), ret)
Copy link
Member

@bkamins bkamins Sep 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a special case someone can pass CartesianIndex as inds - do we want to handle it in the ColumnsRow case or error on it here?

return DictRow(x.names, x.values[st]), st + 1
end

function subset(x::DictRowTable, inds; view::Union{Bool,Nothing} = nothing)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar comments as above to inds apply here

@quinnj quinnj merged commit 2d97b73 into main Sep 6, 2022
@quinnj quinnj deleted the jq/subset branch September 6, 2022 19:42
@ablaom
Copy link
Contributor

ablaom commented Sep 6, 2022

#284 (comment)

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.

3 participants