Allow single column name for cols
#28
Conversation
Codecov Report
@@ Coverage Diff @@
## main #28 +/- ##
==========================================
- Coverage 95.79% 94.54% -1.26%
==========================================
Files 9 9
Lines 119 110 -9
==========================================
- Hits 114 104 -10
- Misses 5 6 +1
Continue to review full report at Codecov.
|
src/linear_combination.jl
Outdated
| @@ -64,6 +64,10 @@ Applies the [`LinearCombination`](@ref) to each of the specified cols in `x`. | |||
| If no `cols` are specified, then the [`LinearCombination`](@ref) is applied to all columns. | |||
| """ | |||
| function apply(x, LC::LinearCombination; cols=nothing) | |||
| if cols isa Union{Symbol, AbstractString} # want same behaviour for single column | |||
There was a problem hiding this comment.
We can use multiple dispatch here instead wich is generally more performant
function apply(x, LC::LinearCombination; cols::Union{Symbol, AbstractString})
apply(x, LC; cols=[cols]
end
There was a problem hiding this comment.
Julia doesn't dispatch on the keyword args though https://docs.julialang.org/en/v1/manual/methods/#Note-on-Optional-and-keyword-Arguments
Methods are dispatched based only on positional arguments, with keyword arguments processed after the matching method is identified.
So you'll need something like a _to_vec method
_to_vec(x::AbstractArray) = x
_to_vec(x) = [x]There was a problem hiding this comment.
So you'll need something like a _to_vec method
Just note I'm trying to allow cols as any collection including Tuple, so I want to dispatch the opposite way on Union{Symbol, AbstractString} rather than AbstractArray.
There was a problem hiding this comment.
Just note I'm trying to allow
colsas any collection includingTuple, so I want to dispatch the opposite way onUnion{Symbol, AbstractString}rather thanAbstractArray.
Maybe just Union{Tuple, AbstractArray} is good enough and clearer, actually.
- `apply` returns the unwrapped column (except LinearCombination) - `apply!` gives the same result
bencottier
force-pushed
the
bc/single-cols
branch
from
f7d31e5 to
7014619
Compare
Closes #26
This is the behaviour using a single column name (i.e. not a list):
applyreturns the single transformed column, "unwrapped"LinearCombination, because it always produces one column anywayapply!allows it, but gives the same result