-
Notifications
You must be signed in to change notification settings - Fork 256
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
Python API and PEP 8 #153
Comments
@utilForever volunteered to work on this issue! @utilForever, currently |
@doyubkim Alright. I'll start working today. |
@utilForever Also checking in to see where we're at on this. Thanks again for taking this effort! |
@doyubkim Thanks. I'll work on this weekend. Please understand that my health is not very good. |
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! |
Same as the other issue: @doyubkim Thanks. I'm OK now. So, please take this issue to me. 😄 |
@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. |
@doyubkim I'm already working on this issue. I'll consider your unifying work. 🎉 |
@utilForever the latest dev branch is now merged into the |
@doyubkim I'm now finished with exception of some grids. However, I'm working on changing the structure from 3 days ago. |
@utilForever thanks for the update! |
@doyubkim Update: My work is almost finished. Can I do a pull request to |
Thanks for the update! Yes, that should be the target branch. |
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.
The text was updated successfully, but these errors were encountered: