Skip to content
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

Use more julian conventions for function names and arguments #155

Open
rafaqz opened this issue Jan 28, 2023 · 6 comments
Open

Use more julian conventions for function names and arguments #155

rafaqz opened this issue Jan 28, 2023 · 6 comments

Comments

@rafaqz
Copy link
Member

rafaqz commented Jan 28, 2023

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
Copy link
Member

visr commented Jan 28, 2023

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
Copy link
Member Author

rafaqz commented Jan 28, 2023

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

@jw3126
Copy link
Member

jw3126 commented Feb 17, 2023

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
Copy link
Member Author

rafaqz commented Feb 17, 2023

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
Copy link
Member

jw3126 commented Feb 18, 2023

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
Copy link
Member Author

rafaqz commented Feb 18, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants