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

Remove calendar dates in RemoveOldCalendarStatements #239

Merged

Conversation

tisseo-deploy
Copy link
Contributor

Summary:

Solve Issue #234 : Remove calendar dates in RemoveOldCalendarStatements

@leonardehrenfried
Copy link
Collaborator

Hi there, I'm the new maintainer of this library.

Do you think you could write a test for this?

@tisseo-deploy tisseo-deploy force-pushed the 234-remove-calendar-dates branch from 32c4502 to 3e2ee06 Compare December 6, 2024 10:38
@tisseo-deploy tisseo-deploy marked this pull request as ready for review December 9, 2024 08:38
@tisseo-deploy
Copy link
Contributor Author

Docs and tests OK, ready for review.

Comment on lines +364 to +371
Additionally, after truncating the calendar entries, it is recommended to use a **retain operation** to ensure that only trips with valid calendar dates are retained.

Without this retain operation, the `trips.txt` file will contain trips with non-existent calendar dates, leading to invalid data.

```
{"op":"transform", "class":"org.onebusaway.gtfs_transformer.impl.RemoveOldCalendarStatements", "remove_today":false}
{"op":"retain", "match":{"file":"calendar_dates.txt"}, "retainBlocks":false}
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is great documentation! ❤️

@leonardehrenfried leonardehrenfried merged commit efaeb7d into OneBusAway:master Dec 9, 2024
5 checks passed
@tisseo-deploy tisseo-deploy deleted the 234-remove-calendar-dates branch December 9, 2024 09:20
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.

4 participants