JuliaGeo / LibGEOS.jl

Julia package for manipulation and analysis of planar geometric objects
MIT License
72 stars 23 forks source link

Use more julian conventions for function names and arguments #155

Open rafaqz opened 1 year ago

rafaqz commented 1 year ago

The method here generally have non-julian names and signatures, making them harder to find and use than is necessary. GeoInterface.jl uses lower case and snake case for methods, and LibGEOS probably should here too.

The widespread use of args rather than kwargs also makes it harder to use. In delaunayTriangluation you would need to specify tol if you want to pass context, instead of those arguments being independent.

delaunayTriangulation(obj::Geometry, tol::Real = 0.0, context::GEOSContext = get_context(obj)) =
    GeometryCollection(delaunayTriangulation(obj.ptr, tol, false, context), context)

Any thoughts on updating everything to something like this?

delaunay_triangulation(obj::Geometry; tol::Real=0.0, only_edges=false, context::GEOSContext=get_context(obj)) =
    GeometryCollection(delaunay_riangulation(obj.ptr; tol, only_edges, context), context)
visr commented 1 year ago

I agree that the latter is nicer. It will be very breaking on course. But perhaps if we are bundling it with the other breaking PRs that are open, and put some joint effort into really polishing up this package, it could be a good idea. We could make it LibGEOS.jl v1.0.

rafaqz commented 1 year ago

Yes, totally breaking. It can be bumped with your other breaking PR.

jw3126 commented 1 year ago

Having more keyword arguments sounds awesome! About camelCase vs snake_case, I like to mirror the naming convention used in the wrapped lib, if the function is just a ccall. If the function is more fancy or opinionated I like to switch to snake_case as @rafaqz suggests.

rafaqz commented 1 year ago

The reason to switch is to unify the ecosystem.

I would like it if ArchGDAL.funcname LibGeos.funcname GeometryBasics.funcname were as much as possible all the same when they do the same thing.

I think that's more useful long term than mirroring the library names.

LibGEOS will soon work directly on all GeoInterface.jl types, it's weird to keep the camel case. (So yes no longer just a ccall)

Otherwise I will write a thin wrapper library that does this in a Julian way and close all my PRs here. It may make more sense to keep this lower level. (Maybe GEOS.jl?)

jw3126 commented 1 year ago

I would like it if ArchGDAL.funcname LibGeos.funcname GeometryBasics.funcname were as much as possible all the same when they do the same thing.

I think that's more useful long term than mirroring the library names.

I agree 100%. Much preferable to conform with julia ecosystem than with C.

I was thinking that https://github.com/JuliaGeo/LibGEOS.jl/blob/master/src/libgeos_api.jl should stay the same. I am happy with more high-level functions using same names as other geometry package.

rafaqz commented 1 year ago

Oh yes exactly! I didn't mean to change the libgeos_api.jl file at all. I should have been clearer.

I meant just the camelCase used outside of that file. Like delaunayTriangulation.