search

xKDR / Survey.jl

Analysis of complex surveys
https://xkdr.github.io/Survey.jl/
GNU General Public License v3.0
53 stars 19 forks source link

Allows multiple columns in domain estimation #250

Closed smishr closed 1 year ago

smishr commented 1 year ago

Allows estimators to take any specification of columns which groupby accepts.

ayushpatnaikgit commented 1 year ago

Perhaps add a test case where by is [:cname, :stype]?

smishr commented 1 year ago

@asinghvi17 do you want to add anything else for this PR?

asinghvi17 commented 1 year ago

No, just some tests. Will get those latest by Monday.

smishr commented 1 year ago

@sayantikaSSG @itsdebartha can you guys add tests using Vector{Symbol} for the domain argument.

@asinghvi17 if you had other tests in mind please list or add here

codecov-commenter commented 1 year ago

Codecov Report

Merging #250 (8a4bf69) into v0.1.1 (9d69475) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            v0.1.1      #250   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          243       242    -1     
=========================================
- Hits           243       242    -1
Impacted Files Coverage Δ
src/by.jl 100.00% <100.00%> (ø)
src/mean.jl 100.00% <100.00%> (ø)
src/total.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.

asinghvi17 commented 1 year ago

I can't commit to this branch, but since I removed all constraints on the index type, we could copy-paste some of the current tests, with the column specifier being Cols(==(colname)) instead of whatever colname was there before. This should test that the passthrough is successful.

smishr commented 1 year ago

I can't commit to this branch

I have added you with write permissions for the repository. You should be able to now.

copy-paste some of the current tests, with the column specifier being Cols(==(colname)) instead of whatever colname was there before. This should test that the passthrough is successful.

This sounds good. @sayantikaSSG @itsdebartha may help in this

asinghvi17 commented 1 year ago

Thanks @smishr! It should, if only because it's a good test of how general the function is.

smishr commented 1 year ago

Thanks @smishr! It should, if only because it's a good test of how general the function is.

right so ideally this (as well as other) functions in the package which currently accept symbol should work with this format as well