[FEATURE] Add travel_time property to water segments - #712
Open
Alex Iannicelli (atiannicelli) wants to merge 1 commit into
Open
[FEATURE] Add travel_time property to water segments#712Alex Iannicelli (atiannicelli) wants to merge 1 commit into
Alex Iannicelli (atiannicelli) wants to merge 1 commit into
Conversation
Adds an optional travel_time property (integer seconds, minimum 1) to the water subtype in both the Pydantic WaterSegment model and the YAML JSON Schema, capturing the scheduled end-to-end crossing time sourced from the OSM duration tag on ferry routes. Includes examples, counterexamples, regenerated JSON Schema baseline, changelog fragment, and a docs note. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Alex Iannicelli <atiannicelli@gmail.com>
Alex Iannicelli (atiannicelli)
added
the
change type - minor 🤏
🗺️ Schema reference docs preview is live!
Note ♻️ This preview updates automatically with each push to this PR. |
Alex Iannicelli (atiannicelli)
requested review from
Dana Bauer (danabauer)
and
a balanced review from Copilot
September 2, 2026 18:17
Copilot started reviewing on behalf of
Alex Iannicelli (atiannicelli)
September 2, 2026 18:17
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The schemas, documentation, generated baseline, and positive and negative examples consistently implement the requested behavior.
Pull request overview
Adds optional ferry crossing duration metadata to water segments so routers can calculate realistic ETAs.
Changes:
- Adds positive integer
travel_timevalidation to both schemas. - Documents the property and supplies examples/counterexamples.
- Regenerates the schema baseline and adds a changelog entry.
File summaries
| File | Description |
|---|---|
schema/transportation/segment.yaml |
Defines water-only travel_time. |
packages/.../segment/water.py |
Adds the Pydantic field and type. |
packages/.../segment_baseline_schema.json |
Updates the generated schema baseline. |
docs/schema/reference/transportation/segment.mdx |
Documents water travel time. |
packages/.../changelog.d/711.feature.md |
Records the feature. |
examples/.../water-ferry.yaml |
Adds a valid schema example. |
reference/examples/.../water-ferry.yaml |
Adds a valid Pydantic example. |
counterexamples/.../bad-travel-time-zero.yaml |
Tests the positive-value constraint. |
counterexamples/.../bad-travel-time-type.yaml |
Tests integer typing. |
counterexamples/.../bad-travel-time-wrong-subtype.yaml |
Tests water-only applicability. |
reference/counterexamples/.../bad-travel-time-zero.yaml |
Tests Pydantic positivity validation. |
reference/counterexamples/.../bad-travel-time-type.yaml |
Tests Pydantic integer validation. |
reference/counterexamples/.../bad-travel-time-wrong-subtype.yaml |
Tests Pydantic subtype validation. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Alex Iannicelli (atiannicelli)
marked this pull request as ready for review
September 2, 2026 18:20
Contributor
|
I believe this should be an extension instead of part of the base schema. My thinking is that we really only have one source for this data that I am aware of, OSM. I would like to see us examine multiple sources and ensure that our schema selection aligns before bringing this in. |
Contributor
Author
|
Are there directions on how to make an extension? Should all speed limit info be converted into an extension? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds an optional
travel_timeproperty tosubtype=watertransportation segments. The property is a positive integer number of seconds capturing the scheduled end-to-end crossing time for the full segment — including time spent docking, loading, and unloading — matching the semantics of the OSMdurationtag on ferry routes it is sourced from.Without it, routers consuming Overture fall back to a default ferry speed (~5 km/h), so car ferries are either avoided or produce wildly inflated ETAs.
Design choices follow the suggestions in the issue (open for working group review on this draft):
minimum: 1) — simplest for consumers.defs.yaml.Changes
packages/overture-schema-theme-transportation/.../segment/water.py: newTravelTimetype (int32,ge=1) and optionaltravel_timefield onWaterSegment.schema/transportation/segment.yaml: newtravelTimeproperty definition and reference from the wateroneOfbranch (replacing the placeholder).segment_baseline_schema.jsonviamake update-baselines.water-ferry.yamland counterexamples (bad-travel-time-zero,bad-travel-time-type,bad-travel-time-wrong-subtype) in bothexamples/+counterexamples/andreference/examples/+reference/counterexamples/.changelog.d/711.feature.md(transportation theme package).docs/schema/reference/transportation/segment.mdxwater subtype tab.Reference
Testing
examples/transportation(pass, including newwater-ferry.yaml) andcounterexamples/transportation(all fail as expected, including the three new water counterexamples) againstschema/schema.yamlwith a draft 2020-12 validator.pytest packages/overture-schema-theme-transportation packages/overture-schema-validation— 474 passed (includes the JSON Schema baseline test and the reference examples/counterexamples).pytest -W error packages/ tests/— 3550 passed; PySpark suite (regenerated expressions including the new field) — 3151 passed under JDK 17.lint-only,mypy-only,docformat-only,doctest-only,check-namespaceall pass.Checklist
Documentation website
Docs preview for this PR.