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

Support Geometry class extension #125

Merged
merged 9 commits into from
Jul 24, 2024
Merged

Support Geometry class extension #125

merged 9 commits into from
Jul 24, 2024

Conversation

jobverplanke
Copy link
Contributor

@jobverplanke jobverplanke commented Jul 24, 2024

This PR adds support for custom geometry classes by extending the current geometry classes.

I was looking for a way to modify some of the array data returned by Point::toArray() method and was unable to do so.
After searching through recent PR's and issues I stumbled upon #63 which actually adds this functionality, but it was closed because the solution was "too complicated and not elegant".

Although this PR does not really differs from the one you already made (#63) I think people can benefit from this functionality, thats why I would like to give it another try.

The only real difference between #63 and this PR is the way of using the custom geometry classes by making it look more "Laravel-ish", for example looking at Laravel Jetstream how it uses custom User, Team, Membership and TeamInvitation models. (https://github.com/laravel/jetstream/blob/5.x/src/Jetstream.php#L282)

Thank you for creating this package!

@MatanYadaev MatanYadaev changed the title feat: Geometry class extension support Support Geometry class extension Jul 24, 2024
@MatanYadaev MatanYadaev merged commit 862f599 into MatanYadaev:master Jul 24, 2024
44 checks passed
@MatanYadaev
Copy link
Owner

@jobverplanke Thanks for pushing this PR!

@jobverplanke
Copy link
Contributor Author

Thank you for merging!

@Mohammed-Alama
Copy link

@jobverplanke I'm really thankful for your PR I have been search for a solution to override Polygon::class to write my own logic for toSqlExpression to use different SRID 4326 but 0 as SRID was not configurable
I have been trying to use bind method
$this->app->bind(Polygon::class, EloquentSpatialPolygonExtension::class);
but it did not work at all because also it was use in Factory
$this->app->bind(\MatanYadaev\EloquentSpatial\Factory::class, EloquentSpatialFactoryExtension::class);
after this investigation I think the package did not use binding/autoloading functionality of laravel
I have curious for your answer , am thinking right or I'm missed something
anyway I will update the package now thank you again ❤️

@MatanYadaev
Copy link
Owner

@Mohammed-Alama Can you try to explain what's your use case? Why did you try to extend the functionality? Maybe I'll figure out a different way to do so.
In addition, I'd appreciate it if you could fork the repo, and create a failing test with the behavior you would like to have. It'll help me understand what you tries to achieve.

@Mohammed-Alama
Copy link

@MatanYadaev I was trying to use 4326 as SRID to be able to index geometry column because I'm using whereContains()
and SRID = 0 it's mean Unknown/undefined coordinate systems
so i was trying to override toSqlExpression to use 4326

firstly try to override Geometry class with $this->app->bind(Geometry::class,GeometryExtension::class)
with no luck

then try to override Polygon::class using the same way since I'm using this one , by extending it at and using at in $casts
but i figure out that it's used in Factory::class but as usual binding / resolving (the laravel way that at least I know)
any of this classes did not work until i found this PR

CleanShot 2024-11-16 at 01 41 36

@MatanYadaev
Copy link
Owner

Did you try the approach mentioned here? https://github.com/MatanYadaev/laravel-eloquent-spatial?tab=readme-ov-file#extend-with-custom-geometry-classes
Using app->bind won't work here, as I don't use Laravel's container in the package.
BTW, I still didn't understand your underlying issue. If you store your geometries as SRID 4326 in the DB, and then you search all the geometries that inside a polygon SRID 4326 using whereContains, it should work fine with SRID 4326 and not SRID 0.

@Mohammed-Alama
Copy link

@MatanYadaev that exactly the issue that you did not use Laravel's container in the package
and after digging for 2 days then check the package I found this PR and this changes
and now I have been using this override method

@Mohammed-Alama
Copy link

I have been through an issue related to use different name from classes form the package
like this EloquentSpatial::usePolygon(EloquentSpatialPolygonExtension::class);
and it fixed with use same name to use arrayToPolygon of GeoJson
EloquentSpatial::usePolygon(Polygon::class);

https://github.com/phayes/geoPHP/blob/685562416ec6d22b9b3927e02ca0ddacf84ca646/lib/adapters/GeoJSON.class.php#L47
CleanShot 2024-11-21 at 01 46 30

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

Successfully merging this pull request may close these issues.

3 participants