-   Notifications  You must be signed in to change notification settings 
- Fork 557
[Java] Python generator implementation #665 #667
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
Conversation
| This PR fails the build. As a minimum it should pass all tests. | 
| Build is still failing with lots of code style issues. | 
| I installed the checkstyle idea plugin imported the style and reformatted everything, but i get tons of errors on indent and line length. Is that an idea issue not respecting fully the checkstyle config? I attached an example of what the IDE does in the second screenshot when reformat is applied with the checkstyle, but then gradle checkstyle reports that as incorrect. | 
| IDEA does not properly support strict indenting. | 
| Got it, okay it'll take me a while to clean them up manually then. I'll push a fix once the checkstyle passes. Meanwhile the rest should work (build, test, etc), without the style. | 
| I think everything is more or less cleaned up, could use more unit tests, but will likely postpone for after initial feedback. | 
| Looks reasonable. You probably want to clean up the copyright and C# references in Javadoc. The major point for me is that is follows the combined encoder and decoder format of the C# generator and not the separation in the Java generator. We've seen this is a much better approach as the separate decoder can deal with read only buffers. | 
| I tend to agree with that point. I already had the issue of having to use mutable buffer types. I'll have a look into how much effort it is to split them. If it turns out to be too big of a rewrite are you OK merging as is contingent on cleanup. Are there any specific edge cases you can think of in general for which it would be a good idea to port the tests? | 
| Have a read back through the issues to get a feel for what to watch out for. | 
| Added some new test cases, additional fixes, the cleanups you suggested and some fixes post rebase. Seems like one commit from Apr 9, diverged the | 
| Just wondering if you had a chance to take a peak at merging that? | 
| As mentioned above we want to move away from the use of a single codec for encode and decode rather than propagate this pattern. Can you look at doing something similar to the Java codec? | 
| Yep I will try and put some cycles towards that, will give you an update when i have a cleaner prototype. | 
| Sorry about the big delay here, got side tracked. This is fully reworked to have enc + dec pattern like the Java version. I used it as the base blueprint. Waiting for travis to complete, let me know how it looks like. | 
| @mjpt777 Wondering if you're still considering merging this upstream or keep it as a fork for Py support? Thx! | 
| Sorry but I had to revert this. As I worked through the code refactoring it I noticed far to many warnings and have concerns about the completeness. Please resubmit with less warnings and try not to have unused methods. | 


This is a very early implementation of a python stub generator. It's derived from the C# Java generator, so it follows most of the code and conventions adopted there.
The
runPythonTestsgradle task will build the python stubs in the project_root/python/tests/gen and run the unit tests. So far only car_example_schema.xml is implemented, but happy to port any other tests that focus on specific edge cases (Maybe a FIX sample).Built and tested with python 3.7, but in practice any version that supports https://docs.python.org/3/c-api/buffer.html should work, so 2.7 should be fine too, although I didn't try it yet (also I much rather not offer support for it generally).
If you think it's not worth it adding and maintaining that code in the mainline I'm happy to fork off only the IR and Java generator as well and providing it as an unsupported 3rd party generator, depending on the discussion.