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

Method v/s manifold #247

Open
asinghvi17 opened this issue Jan 10, 2025 · 4 comments
Open

Method v/s manifold #247

asinghvi17 opened this issue Jan 10, 2025 · 4 comments

Comments

@asinghvi17
Copy link
Member

asinghvi17 commented Jan 10, 2025

It's agreed that we should have

arclength(geom; manifold = discover_manifold(geom), method = nothing) = arclength(method, manifold, geom)

and then

arclength(manifold, geom) = arclength(nothing, manifold, geom)
arclength(::Nothing, manifold, geom) = arclength(BEST_METHOD_FOR_MANIFOLD, manifold, geom)

then we, or users, define methods for the API

@asinghvi17
Copy link
Member Author

This way, we have the ability to disambiguate methods from manifolds.

@rafaqz
Copy link
Member

rafaqz commented Jan 12, 2025

I'm not sure this was what we specifically agreed! :

arclength(method, manifold, geom)

Two traits is a bit confusing and ambiguous for users, another option is that keyword arguments could be preffered and dispatch to traits could be on _arclength, or just one trait would be allowed as the first argument.

There is a point where combinatorial positional traits becomes confusing to read. I'm not sure if two is too many but three definately would be!

@asinghvi17
Copy link
Member Author

Hmm, we could do

arclength(geom; manifold, method)
arclength(manifold, geom; method)

and then dispatch is done internally? I'm still not sure I like the underscore architecture though, it seems a bit fishy for extendability...

Definitely agree on combinatorial position traits being bad. But dispatching off keyword arguments is probably bad form.

I had also considered making algorithms an optional part of this informal "operation interface". But the problem is that you can't shoehorn e.g GEOS() into that framework, so there has to be some form of space for algorithms.

@rafaqz
Copy link
Member

rafaqz commented Jan 12, 2025

Ok right underscore methods are not so good in an interface. It would have to be something else. So I guess its preferable to have those positional methods as long as we have the keyword methods as the main things in the docs.

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

2 participants