-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add rename macro, docs, and tests #343
Changes from 5 commits
9f9e69f
d6f8dcd
1c2d233
3663d39
445be76
15b442e
aafc837
0955d59
756fab4
c53b382
9758250
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -387,6 +387,62 @@ fun_to_vec(ex::QuoteNode; | |
gensym_names::Bool=false, | ||
outer_flags::Union{NamedTuple, Nothing}=nothing) = ex | ||
|
||
|
||
""" | ||
rename_kw_to_pair(ex::Expr) | ||
|
||
Given an expression where the left- and right- hand side | ||
both are both valid column identifiers, i.e., a `QuoteNode` | ||
or an expression beginning with `$DOLLAR`, or a "full" expression of the form | ||
`$DOLLAR(:x => :y)`, return an expression, where expression arguments of type | ||
`QuoteNode`` are converted to `String``. | ||
""" | ||
function rename_kw_to_pair(ex::Expr) | ||
|
||
ex_col = get_column_expr(ex) | ||
if ex_col !== nothing | ||
return ex_col | ||
end | ||
|
||
lhs = let t = ex.args[1] | ||
|
||
s = get_column_expr(t) | ||
if s === nothing | ||
throw(ArgumentError("Invalid column identifier on LHS in DataFramesMeta.jl macro")) | ||
end | ||
|
||
s | ||
end | ||
|
||
rhs = MacroTools.unblock(ex.args[2]) | ||
rhs_col = get_column_expr(rhs) | ||
|
||
if rhs_col === nothing | ||
throw(ArgumentError("Invalid column identifier on RHS in DataFramesMeta.jl macro")) | ||
end | ||
|
||
if rhs_col !== nothing | ||
src = rhs_col | ||
dest = lhs | ||
return :($src => $dest) | ||
end | ||
|
||
end | ||
|
||
function pairs_to_str_pairs(args...) | ||
|
||
map(args) do arg | ||
if !(arg isa Pair) | ||
throw(ArgumentError("Non-pair created in @rename")) | ||
end | ||
|
||
if first(arg) isa Int | ||
return first(arg) => string(last(arg)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for your work. One small thing. I think The problem is that The solution is two-fold
I've done both and added more tests. I've also re-written the docstring. So I think this is good to merge! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, @pdeffebach. Thanks for adding the style fixes, error handling, and adding |
||
end | ||
string(first(arg)) => string(last(arg)) | ||
end | ||
end | ||
|
||
function make_source_concrete(x::AbstractVector) | ||
if length(x) == 1 && x[1] isa AsTable | ||
return x[1] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need the unblock here? If you want to do complicated things, you would still need to hide things behind
$(...)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unblock allows for the use of
begin ... end
in cases such as the following:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't you need a
$
in there though? Since we don't allow arbitrary expressions on the RHS that are not wrapped in$()
. So this seems redundant.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I edited the above to include
$
. If I remove the unblock as in the following:then
throws
Invalid column identifier on RHS in DataFramesMeta.jl macro
. I'll look this over more.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its okay to disallow the second case. People can do