Skip to content

Conversation

@LilithHafner
Copy link
Contributor

This way, if more keyword arguments are added, they will automatically be supported

Seeing these passthrough methods makes me think it might be worth making DataSetColumn <: AbstractVector, but I'm not familiar enough with this package to know if that is a good idea.

This way, if more keyword arguments are added, they will automatically be supported
@sl-solution
Copy link
Owner

Because we want to keep track of datasets' columns, we have not set DatasetColumn as a subtype of AbstractVector. We like to track columns to make sure that if a column is changed by something rather than IMD's APIs, its parent's metadata is updated (for example if a dataset is sorted by the modified column, then its sorting status must be removed).

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2022

Codecov Report

Merging #95 (2e3e6a5) into master (44445f6) will increase coverage by 0.05%.
The diff coverage is 85.45%.

@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
+ Coverage   60.89%   60.95%   +0.05%     
==========================================
  Files          51       51              
  Lines       11241    11245       +4     
==========================================
+ Hits         6845     6854       +9     
+ Misses       4396     4391       -5     
Impacted Files Coverage Δ
src/abstractdataset/dscol.jl 14.06% <0.00%> (+0.32%) ⬆️
src/sort/util.jl 48.68% <55.55%> (ø)
src/dataset/constructor.jl 98.26% <100.00%> (ø)
src/dataset/modify.jl 45.56% <100.00%> (ø)
src/sort/qsort.jl 98.49% <100.00%> (+0.05%) ⬆️
src/sort/sort.jl 78.77% <100.00%> (ø)
src/join/join_dict.jl 86.02% <0.00%> (+0.43%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sl-solution sl-solution merged commit e3f37c4 into sl-solution:master Oct 25, 2022
@LilithHafner LilithHafner deleted the patch-3 branch October 25, 2022 10:58
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