LibGEOS.jl icon indicating copy to clipboard operation
LibGEOS.jl copied to clipboard

Use more julian conventions for function names and arguments

Open rafaqz opened this issue 2 years ago • 6 comments

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)

rafaqz avatar Jan 28 '23 11:01 rafaqz

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.

visr avatar Jan 28 '23 18:01 visr

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

rafaqz avatar Jan 28 '23 19:01 rafaqz

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.

jw3126 avatar Feb 17 '23 10:02 jw3126

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?)

rafaqz avatar Feb 17 '23 12:02 rafaqz

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.

jw3126 avatar Feb 18 '23 05:02 jw3126

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.

rafaqz avatar Feb 18 '23 10:02 rafaqz