Skip to content
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

tensorflow: bind DotGraph #272

Merged
merged 1 commit into from
Aug 31, 2016

Conversation

Mistobaan
Copy link
Contributor

@Mistobaan Mistobaan commented Aug 18, 2016

This is a first attempt at getting more familiar with javacpp and the tensorflow bindings. Not sure if is the best way to map this simple func + struct, but it compiles and works .

@saudet
Copy link
Member

saudet commented Aug 21, 2016

Yes, I think that's pretty much the best we can do. Good job!

The ordering and formatting of the new code could be a bit more consistent with the existing code, but it's not a big deal.

Let me know if you intend to work more on this or not! Thanks

@Mistobaan
Copy link
Contributor Author

sure I can fix the formatting. Is there a defined format for the project that I can just run and auto formats it ? (like go fmt).

I am not able to "subclass" those Function Pointers to actually implement a Java-struct to pass to inside the C++ Dot function. Even adding @Virtual in the code. Maybe I am doing it wrong. How would you subclass it properly ?

@saudet
Copy link
Member

saudet commented Aug 28, 2016

No, don't have a formatter for that. Like I said, it's still alright the way it is now :)

When you say you can't subclass them, what problems are you encountering? Simply overriding a call() method and letting it use the protected constructor should work. What is the expected behavior vs the observed one?

@saudet
Copy link
Member

saudet commented Aug 31, 2016

In any case, it builds fine for me, so let's merge this in. If you still have problems getting callbacks to work, let me know! Thanks

@saudet saudet merged commit 2ecc400 into bytedeco:master Aug 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants