Fix #328 parsing bug try 2

[pdeffebach]

Just takes the bugfix from #341

I cluttered up #341 with docstrings, and the diff is actually a little important.

Ideal scenario: create Expr(., f)

This works on 1.6+ but tests currently pass on 1.0 and 1.5. Including this fix would require changing compatability. This is a bug-fix so I don't want to do that.

Current scenario: use $ByRow($f)

This is not should to Expr(., f) except for extremely contrived edge cases which I do not know about.

Once we release 1.0, we will depend on DataFrames.jl version which require 1.6 and above, and therefore depend on 1.6.

The docstrings and explanations will be added in a later PR.

[pdeffebach]

@bkamins Let's merge this and add docs separately.

[bkamins]

Looks almost good. This:

julia> x = [true, false]
2-element Vector{Bool}:
 1
 0

julia> y = .-(.+(x))
2-element Vector{Int64}:
 -1
  0

julia> df = DataFrame(x=x)
2×1 DataFrame
 Row │ x
     │ Bool
─────┼───────
   1 │  true
   2 │ false

julia> @select df :y = .-(.+(:x))
ERROR: UndefVarError: .- not defined

fails, but maybe we are OK, as it is a corner case.

The only situation where I think it could be useful is negation with !, e.g.:

julia> df = DataFrame(x=[true, false, false])
3×1 DataFrame
 Row │ x
     │ Bool
─────┼───────
   1 │  true
   2 │ false
   3 │ false

julia> @select df :y = (x -> .!(x))(reverse(:x))
3×1 DataFrame
 Row │ y
     │ Bool
─────┼───────
   1 │  true
   2 │  true
   3 │ false

julia> @select df :y = .!(reverse(:x))
ERROR: UndefVarError: .! not defined

[pdeffebach]

Thank you! I was missing a fix_simple_dot somewhere.

Ultimately, though, we actually have to change support to only include 1.6+. See the following

julia> ByRow(+)(1)
ERROR: MethodError: no method matching (::ByRow{typeof(+)})(::Int64)
Closest candidates are:
  (::ByRow)() at ~/.julia/packages/Tables/fouQB/src/utils.jl:241
  (::ByRow)(::NamedTuple{(), Tuple{}}) at ~/.julia/packages/Tables/fouQB/src/utils.jl:242
  (::ByRow)(::NamedTuple{names, T} where {N, names, T<:Tuple{Vararg{AbstractVector, N}}}) at ~/.julia/packages/Tables/fouQB/src/utils.jl:234
  ...
Stacktrace:
 [1] top-level scope
   @ REPL[20]:1

julia> .+(1)
1

This comes into play when we see

@select df :z = .+(first(:x))

Where we have

julia> .+(first([1]))
1

julia> ByRow(+)(first([1]))
ERROR: MethodError: no method matching (::ByRow{typeof(+)})(::Int64)
Closest candidates are:
  (::ByRow)() at ~/.julia/packages/Tables/fouQB/src/utils.jl:241
  (::ByRow)(::NamedTuple{(), Tuple{}}) at ~/.julia/packages/Tables/fouQB/src/utils.jl:242
  (::ByRow)(::NamedTuple{names, T} where {N, names, T<:Tuple{Vararg{AbstractVector, N}}}) at ~/.julia/packages/Tables/fouQB/src/utils.jl:234
  ...
Stacktrace:
 [1] top-level scope
   @ REPL[23]:1

so I need to lower things down to Expr(., f) and remove support for < 1.6.0

[bkamins]

Yes, support for Julia 1.6 can be required.

Also you are right, by requiring to use . not ByRow. The reason, again, corner cases, is that ByRow has special meaning for NamedTuple also.

As for tests - maybe add some tests with data frames having more than 1 row.

[pdeffebach]

Okay tests pass with this change. I think we should merge and then I can add the docs from #341 later.