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

MDS 2.0 - accessibility_options for vehicles should be optional not required #847

Closed
lastalfriday opened this issue Apr 25, 2023 · 12 comments
Labels
Data Types Changes to the base data_types.md file Schema Implications for JSON Schema or OpenAPI
Milestone

Comments

@lastalfriday
Copy link

lastalfriday commented Apr 25, 2023

The title says it all :)

https://github.com/openmobilityfoundation/mobility-data-specification/blob/2.0.0-rc1/data-types.md#vehicles

image

@thekaveman thekaveman added this to the 2.0.0 milestone Apr 25, 2023
@thekaveman thekaveman added the Schema Implications for JSON Schema or OpenAPI label Apr 25, 2023
@schnuerle
Copy link
Member

Probably "Required if Available" is the way to go.

@thekaveman
Copy link
Collaborator

@schnuerle how about for Trips? accessibility_options is also listed as required there, should it be moved to "Required if Available"? https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/data-types.md#trips

@lastalfriday
Copy link
Author

Ok, I never really understood the idea behind "Required if available", but I'm guessing it has to do with encouraging providers to supply it if they have it..

Anyway, in the database it won't make a difference for me, it will still need to be "nullable".

"Naming things" is the hardest part of IT, so I have full understanding for this, but when you name something xxx_options and it's not optional, well...

@thekaveman
Copy link
Collaborator

"Naming things" is the hardest part of IT, so I have full understanding for this, but when you name something xxx_options and it's not optional, well...

Agreed on "naming things" being difficult!! Though "options" here is being used as a synonym for "choices" - unrelated to "optional" / "required".

@lastalfriday
Copy link
Author

lastalfriday commented Apr 27, 2023

Ok, I get it. Confusing though :)
..could you not use "attributes" => accessibility_attributes ? ...since it's used other places..

I also found a couple of other properties that should not be required, but optional or "required if available".
It makes no sense for a required property to have the value null

image

I also see that this is correct for events, so..

image

@lastalfriday
Copy link
Author

lastalfriday commented Apr 27, 2023

A (non-related) question regarding trip_ids defined for telemetry:

I'm trying to make sense of this, but just can't...

In what scenario does a telemetry point belong to two trips ?

..and if there is such a scenario, why doesn't it apply to the journey as well?

@schnuerle
Copy link
Member

schnuerle commented Apr 27, 2023

accessibility_options is really meant to list the options available for accessibility. It doesn't have anything to do with if the field is optional! You can have accessibility options available or not, so listing them is optional.

But I do think accessibility_attributes may be a better term here. I can make this change throughout.

For the side question of telemetry on multiple trips, the scenario came up in the working groups where the same trip in a car could be used to transport goods and/or people at the same time, under different billing and different customers. So trips can overlap within a mode or across multiple modes (like maybe food delivery at the same time as passenger delivery). It may apply for the journey as well, but I think this can roll up through the trips if needed, and trips have more attributes than journeys at the moment.

thekaveman added a commit to openmobilityfoundation/mds-openapi that referenced this issue Apr 27, 2023
@lastalfriday
Copy link
Author

lastalfriday commented Apr 28, 2023

Thanks for changing to accessibility_attributes, that's much more clear

Ok, I guess that could be some kind of a scenario, even though I still have hard time seeing this as a real world scenario using mds, but that could ofc just be me being too deep in our own use cases.

..though I'm struggling to see how this many2one should be implemented...

Lets say we have: 2 trips that share 1 telemetry object like you describe

From the provider's perspective:

  1. They must first create 2 incomplete trip objects in their system to get trip_ids that will be needed to send ongoing telemetry (they're incomplete because the trips are not done)
  2. These trip objects will only exist in their system (the trips are not done, so cannot send to the agency yet)
  3. They send telemetry to the agency with these trip_ids
  4. When the trips are done, they send the trips to the agency

From the agency's perspective:

  1. We receive telemetry points for trips that does not yet exist in our system
  2. We later receive these trips (if all goes well...)

Is this how it will work?

Even still... the "required" bit (explained with screenshots) should be changed to match events though?

@schnuerle
Copy link
Member

I've got accessibility_options changed to accessibility_attributes in all places in the spec with this commit. With review by @thekaveman we can close this issue.

@lastalfriday if you want to continue discussing the overlapping trips telemetry, which I think warrants discussion, please open a new issue and include your summary and screenshots!

@schnuerle schnuerle added the Data Types Changes to the base data_types.md file label May 2, 2023
@thekaveman
Copy link
Collaborator

In reviewing Policy, I noticed there's a reference to Accessibility Options in the policy.rule: https://github.com/openmobilityfoundation/mobility-data-specification/blob/release-2.0.0/policy/README.md#rules

In addition to changing the field name there, it might make sense to elevate this field out of Rule and up to the Policy document. Since the mode_id is also at the Policy document level, and accessibility_attributes depend on mode_id.

@lastalfriday
Copy link
Author

@schnuerle
Thank you for renaming to accessibility_attributes. I see the changes live now in the release-2.0.0 branch.

However! I almost forgot this as we started discuss other things, my main point here was that this field should not be "Required", but either "Optional" or "Required if Available" (as you yourself suggested early on in this thread)

@schnuerle
Copy link
Member

Thanks for the catch this is cleaned up here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Data Types Changes to the base data_types.md file Schema Implications for JSON Schema or OpenAPI
Projects
None yet
Development

No branches or pull requests

3 participants