Changes to data structures #407
Conversation
Changes objects vehicle_type_dock_capacity and vehicle_type_area_capacity to arrays.
formatting fixes to JSON examples
|
This discussion has been automatically marked as stale because it has not had recent activity. It will be closed in 60 days if no further activity occurs. Thank you for your contributions. |
|
Entur supports this change |
|
@mplsmitch How can we move this forward? Call to vote? |
|
Hello @testower - the next step is to call a vote on this along with #454, #457 and #460 and anything else we'd like to get into the v3.0 release. In the past we've made the most progress if we group the votes an a number of issues, as opposed to holding individual votes. The PRs need to be open for comment for 7 days before a vote can be called. I'd like to do something about #306, basically it would be like your file: https://api.entur.io/mobility/v2/gbfs/ Great work on the Internationalization! |
|
I hereby call a vote on this proposal. Voting will be open for 10 full calendar days until 11:59PM UTC on Friday, January 20th. Please vote for or against the proposal, and include the organization for which you are voting in your comment. Please note if you can commit to implementing the proposal. |
|
+1 from Transit |
|
+1 from Entur |
gbfs.md
Outdated
| "abc123": 7, | ||
| "def456": 9 | ||
| } | ||
| "is_charging_station": "true", |
There was a problem hiding this comment.
did you mean to stringify true here?
There was a problem hiding this comment.
Probably a mistake - will fix when/if we merge.
There was a problem hiding this comment.
Dumb question, but why not fix it now? Isn't it better to be precise on what we are voting for?
gbfs.md
Outdated
| \- `vehicle_type_area_capacity` <br/>*(added in v2.1)* | OPTIONAL | Object | An object used to describe the parking capacity of virtual stations (defined using the `is_virtual_station` field), where each key is a `vehicle_type_id` as described in [vehicle_types.json](#vehicle_typesjson) and the value is a number representing the total number of vehicles of this type that can park within the virtual station.<br/><br/>If the virtual station is defined by `station_area`, this is the number that can park within the station area. If `lat`/`lon` is defined, this is the number that can park at those coordinates. | ||
| \- `vehicle_type_dock_capacity` <br/>*(added in v2.1)* | OPTIONAL | Object | An object used to describe the docking capacity of a station where each key is a `vehicle_type_id` as described in [vehicle_types.json](#vehicle_typesjson) and the value is a number representing the total docking points installed at this station, both available and unavailable for the specified vehicle type. | ||
| \- `vehicle_type_area_capacity` <br/>*(as of v3.0)* | OPTIONAL | Array| This field's value is an array of objects containing the keys `vehicle_type_id` and `count` defined below. These objects are used to model the parking capacity of virtual stations (defined using the `is_virtual_station` field) for each vehicle type defined in `vehicle_types.json`. | ||
|   \- `vehicle_type_id`| Conditionally REQUIRED | ID | A `vehicle_type_id`, as defined in `vehicle_types.json`, that may park at the virtual station. |
There was a problem hiding this comment.
Dumb question, but why is this "Conditionally Required"? Under what conditions would it be left empty? Same for count field below.
There was a problem hiding this comment.
They are required if vehicle_type_area_capacity is used, if not, they are not required. We can clarify this when/if we merge.
There was a problem hiding this comment.
What is the convention in the spec for a field that is required if its parent element is specified, where the parent element may be optional? If I understand correctly, in this PR the required child fields are being marked as "Conditionally Required". However, I see in PR #460 that newly added child fields are being marked simply as "Required", even if there parent element is optional. I'll admit the latter convention seems more natural to me, but understood if that's not the convention used in the spec.
There was a problem hiding this comment.
Sorry for the delayed response, I've been traveling and just got back. I'll update the PR with corrections. @bdferris-v2 you have a good point, we need to be consistent on how parent-child relationships are described throughout. I think you are correct that they should be labeled simply as required in this case because we could have parent-child relationships with other dependencies making them conditionally required. Making these consistent throughout the spec wouldn't change the behavior of any fields so it won't require a vote. We should just go ahead and do it now so it can get rolled into the next release.
Some day I want to write a style guide that would contain all these conventions.
Change child fields of optional parent from Conditionally Required to Required. Change incorrect string to boolean. Needed for PR #457
|
Mitch Vars seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Voting on this PR closes in 2 calendar days. Please vote for or against the proposal, and include the organization for which you are voting in your comment. Please note if you can commit to implementing the proposal. |
* Change child field requirements, remove string Change child fields of optional parent from Conditionally Required to Required. Change incorrect string to boolean. * fix typo in vehicle_type_dock_capacity
|
+1 for harmonization of data structure from nextbike |
|
+1 form Lime |
|
+1 from Joyride |
|
+1 from BCycle |
|
This vote has now closed, and it passes! Votes in favor: |
What problem does your proposal solve? Please begin with the relevant issue number. If there is no existing issue, please also describe alternative solutions you have considered.
We have two fields,
vehicle_type_area_capacityandvehicle_type_dock_capacitythat use data from other fields as object keys. This is inconsistent with other similar fields in the specification such asvehicle_types_availableand has caused some challenges in generating models and validation.What is the proposal?
This proposal changes the objects to arrays as was done with
vehicle_types _available. I've read through the rest of the specification and haven't found this issue elsewhere. If anyone is aware other examples please chime in so we can roll them into this PR.Is this a breaking change?
Which files are affected by this change?