Add code to generate signature from Method #1
Conversation
|
Not tests show that it works for VarArgs |
Codecov Report
@@ Coverage Diff @@
## master #1 +/- ##
==========================================
+ Coverage 94.93% 98.58% +3.64%
==========================================
Files 2 4 +2
Lines 79 141 +62
==========================================
+ Hits 75 139 +64
+ Misses 4 2 -2
Continue to review full report at Codecov.
|
|
Turns out this does not work in Julia 1.0, because I access a field of |
oxinabox
changed the title
oxinabox
changed the title
src/method.jl
Outdated
| return whereparams | ||
| end | ||
|
|
||
| function parameters(meth::Method) |
There was a problem hiding this comment.
It bothers me a bit that this shared a name with the other parameters method.
Though I guess they do the same thing.
One option would be to rename this to params to match the dict key it is filling
This only happens in 1.0 I am tempted to just disable this test in 1.0 |
Co-Authored-By: Nick Robinson <[email protected]> Co-Authored-By: Curtis Vogt <[email protected]>
Co-Authored-By: Curtis Vogt <[email protected]>
Co-Authored-By: Nick Robinson <[email protected]>
|
How close is this to being ready for merge @omus ? |
I was mainly waiting for the tests to be fixed. I think we're pretty close to merging. I'll do another review today but I think we're in pretty good shape. |
| haskey(target, :params) && @test target[:params] == get(candidate, :params, nothing) | ||
| haskey(target, :args) && @test target[:args] == get(candidate, :args, nothing) | ||
| haskey(target, :whereparams) && | ||
| @test target[:whereparams] == get(candidate, :whereparams, nothing) |
There was a problem hiding this comment.
Should we add tests for the other fields that splitdef returns and just use @test_broken?
There was a problem hiding this comment.
I don't think it is required. Do you think it is?
Right now we are matching what we have documented it does I would.
Should we save test broken for actual bugs?
There was a problem hiding this comment.
We should do @test !haskey(candidate, :rtype) to ensure these aren't set which is what we expect. If someone attempts to get these working then the test will fail and they will need to update these tests.
There was a problem hiding this comment.
i became convinced and added @test_broken
Co-Authored-By: Curtis Vogt <[email protected]>
Co-Authored-By: Curtis Vogt <[email protected]>
|
|
||
| Limitted support for: | ||
| - `:kwargs`: Keyword arguments of the function. | ||
| Only the names will be included, not the default values or type constraints. |
There was a problem hiding this comment.
Adding this much support is exactly as hard as detecting if there are any or not.
There was a problem hiding this comment.
as far as I can see all of @omus's comments have been addressed, and tests pass (and are thorough) 👍
Co-Authored-By: Nick Robinson <[email protected]>
Co-Authored-By: Nick Robinson <[email protected]>
|
Thanks @nickrobinson251 |
Goal is to generate a dictionary like is returned from
splitdefwhen given only aMethodobject.So that you can manipulate it,
and then call
combinedefon the result, to get the AST of the code you want to eval.