-
Notifications
You must be signed in to change notification settings - Fork 235
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
Dart lang support #167
Dart lang support #167
Conversation
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.
@MattConflitti Great contribution! And extremely prompt, BTW 😄 . Please check my comments below.
@MattConflitti thank you so much for this PR 🥇 💪 |
Great! Then the tests pass and this PR should be pretty much set as far
as I can tell! Thanks for your help.
…On Thu, Feb 20, 2020, 10:51 AM Iaroslav Zeigerman ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In m2cgen/exporters.py
<#167 (comment)>:
> @@ -251,6 +251,29 @@ def export_to_php(model, indent=4):
return _export(model, interpreter)
+def export_to_dart(model, class_name="Model", indent=4):
Should I use the subroutine mixin as well?
No as long as there are no restrictions on a size of the individual
function/method in DART.
Is that necessary if the bin threshold works up to 465 for Dart?
Not really. BinExpressionDepthTrackingMixin and the Subroutine mixin
address somewhat different problems. The former workarounds the constraints
on the depth of binary expressions while the later helps overcome
function/method size constraints.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#167?email_source=notifications&email_token=ADBVIG2K5ED654FBEYLAR3TRD2RHXA5CNFSM4KVZZ5J2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCWJ7AAY#discussion_r382088746>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBVIGZ2I54576QNH52IVVLRD2RHXANCNFSM4KVZZ5JQ>
.
|
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.
@MattConflitti Please resolve a merging conflict and add function_name
argument to the Dart interpreter which was introduced recently in #166.
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.
@MattConflitti Thanks a lot for the awesome job! LGTM!
@StrikerRUS @izeigerman Thank you for your help and maintaining this repo! |
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.
@MattConflitti great job, thank you 🥇
@MattConflitti Can you please generate code examples for Dart in a follow-up PR? m2cgen/tools/generate_code_examples.py Lines 34 to 45 in 3c431f2
and run the following comand Line 15 in 3c431f2
|
Absolutely!
…On Mon, Feb 24, 2020, 2:30 PM Nikita Titov ***@***.***> wrote:
@MattConflitti <https://github.com/MattConflitti> Can you please generate code
examples
<https://github.com/BayesWitnesses/m2cgen/tree/master/generated_code_examples>
for Dart in a follow-up PR?
Add Dart here
https://github.com/BayesWitnesses/m2cgen/blob/3c431f2c91c55cffe075c600efdcd9730465918c/tools/generate_code_examples.py#L34-L45
and run the following comand
https://github.com/BayesWitnesses/m2cgen/blob/3c431f2c91c55cffe075c600efdcd9730465918c/Makefile#L15
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#167?email_source=notifications&email_token=ADBVIG2K3JVILVTKYXIHQJDREQN6DA5CNFSM4KVZZ5J2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMZG75Q#issuecomment-590508022>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBVIGZ4LJUPG5TWWHJFTFLREQN6DANCNFSM4KVZZ5JQ>
.
|
Issue of tanh function is still there. There is currently a unit test that expects the use of tanh to throw an error.
Edit: tanh issue is fixed thanks to guidance from @StrikerRUS