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

New strategy : Retain up from polygon #310

Conversation

tisseo-deploy
Copy link
Contributor

Summary:

Solve Issue #237 : Add RetainUpFromPolygon strategy

@CLAassistant
Copy link

CLAassistant commented Dec 10, 2024

CLA assistant check
All committers have signed the CLA.

@tisseo-deploy
Copy link
Contributor Author

Ready for review We will add a full exemple of the config used at Tisséo with the next (and last) PR

@@ -270,6 +271,26 @@ You can remove a specific entity from a feed.

Note that removing an entity has a cascading effect. If you remove a trip, all the stop times that depend on that
trip will also be removed. If you remove a route, all the trips and stop times for that route will be removed.

#### Retain Up From Polygon
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must admit that I find the terminology of "retaining up" and "retaining down" confusing. What do you mean by that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We did'nt found a better term. This one is direct from the existing retain algotithm that seems very stable.
"Up" means that the algorithm will traverse up the entity dependency tree by applying retainUp to each of them. Starting from the stop, retainUp will be applied to the stop_times referencing this stop, then to the trips, and so on.
Then, when the base of the tree is reached, it applies retainDown to all the traversed entities. Therefore, all the trips of the route and then all the stop_times of each trip will be tagged as "retain".
Entities not marked as "retain" will be deleted.
The goal of the strategy is to retain all entities that are directly or indirectly linked to the area.

We will add this brief explanation to the documentation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, that's fine.


@Override
public void run(TransformContext transformContext, GtfsMutableRelationalDao gtfsMutableRelationalDao) {
Geometry geometry = buildPolygon(polygon);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this into the setter and check if it's valid and non-empty there. Throw an exception immediately if it isn't.

Comment on lines 77 to 79
String message = String.format("Error parsing WKT string : %s", e.getMessage());
log.error(message);
return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't cover up a bad input string and re-throw as a RuntimeException. The program should fail as soon as possible.

* @return true if the point is within the boundaries of the geometry; false otherwise.
*/
private boolean insidePolygon(Geometry geometry, double lon, double lat) {
GeometryFactory geometryFactory = new GeometryFactory();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wil merge this in and then make this an instance of the class instead.

@leonardehrenfried
Copy link
Collaborator

I will merge this and make a few small changes.

@leonardehrenfried leonardehrenfried merged commit bf32f91 into OneBusAway:master Dec 12, 2024
5 checks passed
@SeifGhz
Copy link
Contributor

SeifGhz commented Dec 12, 2024

Update documentation and code (exception handling has been fixed)

@tisseo-deploy tisseo-deploy deleted the 237-RetainUpFromPolygon-strategy branch December 12, 2024 13:14
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.

5 participants