codedthinking / Kezdi.jl

An umbrella of Julia packages for data analysis, in loving memory of Gábor Kézdi
Other
9 stars 0 forks source link

parse and use `@if true` properly #50

Closed gergelyattilakiss closed 5 days ago

gergelyattilakiss commented 1 week ago

Constant conditions are not working well

gergelyattilakiss commented 1 week ago

booleans are fixed in https://github.com/codedthinking/Kezdi.jl/commit/0c5540534e0bf1b57f08bce9f7b08c8aa2183029

gergelyattilakiss commented 1 week ago

constant conditions are a bit more difficult, they are expressions, but in the bitmask they are evaluated as booleans that can not be row condition.

gergelyattilakiss commented 1 week ago

fixed in 0bf74c7d3e457fe74aec30aabbf0c7c49dbbb918 , used variables do not make a problem because @if only looks for variabels in the dataframe, and later if we add context with scalars(...), then in scalars too.

korenmiklos commented 1 week ago

Now I see the problem. If the condition returns a vector, we get a BitVector that can be used to index the dataframe.

If the condition is a scalar, it cannot be used for indexing.

What if we just add a vector of 0s? Automatic broadcasting should turn the scalar into a vector, and we are fine.

korenmiklos commented 1 week ago

BitVector(zeros(Bool, n)) .| bitmask

gergelyattilakiss commented 6 days ago

Will start working on it.

korenmiklos commented 6 days ago

No, that's the beauty. Broadcasting is automatic and false | x is equal to x. The only difference is that BitVector .| x will be Vector shaped.

korenmiklos commented 6 days ago
julia> BitVector(zeros(5)) .| false
5-element BitVector:
 0
 0
 0
 0
 0

julia> BitVector(zeros(5)) .| true
5-element BitVector:
 1
 1
 1
 1
 1

julia> BitVector(zeros(5)) .| [1, 0, 1, 1, 0]
5-element Vector{Int64}:
 1
 0
 1
 1
 0
gergelyattilakiss commented 6 days ago

Yepp, I started testing these in terminal and see how it goes.

korenmiklos commented 6 days ago

Make sure to put $bitmask in () to ensure operator precedence.

gergelyattilakiss commented 6 days ago

Do we prefer inline solution or should I put the indexing vector into a different variable?

gergelyattilakiss commented 6 days ago

It took me sometime to find the proper place for the broadcasting... but it seems to work fine now.

korenmiklos commented 6 days ago

I'd rather add this to build_bitmask

korenmiklos commented 6 days ago

Please move to build_bitmask. This will be easier to maintain and to merge with other code.

Also add at least 3 tests for this condition, like if true and if 2 + 2 == 4 and if 1 < 0