doyubkim / fluid-engine-dev

Fluid simulation engine for computer graphics applications
https://fluidenginedevelopment.org/
MIT License
1.9k stars 265 forks source link

Python API and PEP 8 #153

Open doyubkim opened 6 years ago

doyubkim commented 6 years ago

Current Python API follows corresponding C++ API naming convention which partially breaks PEP 8 (ex. function names). Other frameworks such as TensorFlow for instance, seems to follow language-specific guidelines. That would be ideal for Jet framework as well. However, changing the naming convention will break the API obviously, so it should target for v2.

doyubkim commented 6 years ago

@utilForever volunteered to work on this issue!

@utilForever, currently dev-v2-gpu is the main dev branch for the v2 release. This contains new C++ APIs (such as new vector/matrix implementations) so it would be great to start from there. You can branch out from dev-v2-gpu and create something like dev-v2-gpu-pep8, make changes, and raise PR against dev-v2-gpu (not the master of course).

utilForever commented 6 years ago

@doyubkim Alright. I'll start working today.

doyubkim commented 6 years ago

@utilForever Also checking in to see where we're at on this. Thanks again for taking this effort!

utilForever commented 6 years ago

@doyubkim Thanks. I'll work on this weekend. Please understand that my health is not very good.

doyubkim commented 6 years ago

Same as the other issue: @utilForever I am terribly sorry to hear that :( You should take a good break and not look at any codes. I will take this issue and raise a PR. Please take a good rest and get well soon!

utilForever commented 6 years ago

Same as the other issue: @doyubkim Thanks. I'm OK now. So, please take this issue to me. 😄

doyubkim commented 5 years ago

@utilForever I am currently working on unifying redundant 2-D and 3-D codes and this will introduce quite a lot of changes in the Python binding lib. If you haven't started working on this issue, I recommend not starting it until the unifying work is completed.

utilForever commented 5 years ago

@doyubkim I'm already working on this issue. I'll consider your unifying work. 🎉

doyubkim commented 5 years ago

@utilForever the latest dev branch is now merged into the dev-v2-gpu branch. This contains some breaking changes in Python APIs so you may need to adapt to it. What's the latest status of this PEP8 work?

utilForever commented 5 years ago

@doyubkim I'm now finished with exception of some grids. However, I'm working on changing the structure from 3 days ago.

doyubkim commented 5 years ago

@utilForever thanks for the update!

utilForever commented 5 years ago

@doyubkim Update: My work is almost finished. Can I do a pull request to dev-v2-gpu branch?

doyubkim commented 5 years ago

Thanks for the update! Yes, that should be the target branch.