-
Notifications
You must be signed in to change notification settings - Fork 23
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
Extend code generation #165
Comments
I never really planned to generate most of If it makes it easier to maintain this package and keep it synchronized with GEOS, I'm all for. With eval types there is a risk that the code will become harder to read, so it needs to be worth it, can't say now. |
The eval is maybe less of a win due to readability, true. One good point to it is it forces you to be explicit about any differences between objects. In some ways that's easier to maintain than repeated slightly different code, even if its hard to read. What were you talking about generating more of here then? Maybe I misunderstood your comment. (Also my reasons were mostly thinking about how little dev effort there is for such a large interface here, and adding more code can just spread the maintenance too thin - generating it wouldn't) |
Yeah good points. Definitely not against. And better docs including developer docs can also help to keep it accesible. Honestly I don't remember what I said about code generation. |
The more I look at the code here the more it seems most of the package could be generated along with the existing generation of the low level api functions.
We can add user facing method generators with ast transformations in the generator - 70% of geos_functions.jl could be defined like this without changes.
We could even generate keyword argument versions and geointerface methods using pretty simple transformation rules, and do name changes to e.g. match GeoInterface.jl function names, using a lookup table.
The types could mostly be defined in an
@eval
loop.@visr you have mentioned code generation here at times, was this what you meant?
The text was updated successfully, but these errors were encountered: