-
Notifications
You must be signed in to change notification settings - Fork 15
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
Complete PFA DSL Library #13
base: master
Are you sure you want to change the base?
Conversation
@@ -57,6 +58,19 @@ class CoreLibrarySuite extends FunctionLibrarySuite { | |||
assert(engine.action(engine.jsonInput("4.0")) == 2.0) | |||
} | |||
|
|||
test("Core divfloor") { |
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.
These tests were added by hand before i did the code gen thing
@@ -1,7 +1,7 @@ | |||
package com.ibm.aardpfark.pfa.expression | |||
|
|||
import com.ibm.aardpfark.pfa.document.{PFAExpressionSerializer, ParamSerializer, SchemaSerializer} | |||
import org.json4s.native.JsonMethods.parse | |||
import org.json4s.native.JsonMethods.{parse => parseJSON} |
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 dsl._ namespace has parse.{int,float..}
functions that conflict with JsonMethods
I ran into a bunch of conflicts where either:
Overall, I would like to understand whether the DSL should be completely loosely types, or as strongly types as possible, and that's future work we should include |
The build is failing because of some maven key thing, any insight into how to fix it? |
Additionally there are PFAExpressions that were peppered through that provide implicit casting of strings to StringLiterals, so some tests are failing. Waiting to hear on the above to resolve it |
@MLnick any insight into the above questions? |
Hi there - I am out of office currently but will take a look early next
week and revert. Thanks for your efforts!
…On Wed, 2 Jan 2019 at 22:05, Paxanator ***@***.***> wrote:
@MLnick <https://github.com/MLnick> any insight into the above questions?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#13 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA_SB2g72LA-YhJ2WFEzQl18MBIM7-L7ks5u_RD8gaJpZM4YsTIL>
.
|
@Paxanator overall the approach looks good and I like the code gen for functions. It would be interesting to also see whether we could code gen the test cases based on the PFA conformance tests JSON (but that may be more involved). Overall ideally I think the DSL would be as strongly-typed as possible, to try to avoid runtime errors at conversion time (i.e. generating invalid PFA, that can only be caught running integration / conformance tests or trying to execute the PFA). As you mention it's not very close to that goal as yet but would be good to work towards it. This is why many of the functions (in most cases the ones we added later on in development) try to take Of course, this does introduce more "magic" and there's an argument that being more explicit and requiring users to use |
Address #7