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

Epiap accessibility path link enhancement #538

Closed

Conversation

Aurige
Copy link
Contributor

@Aurige Aurige commented Nov 3, 2023

Enhancements of the Pathlink proposed by the French team working on an accessibility capture tool

These enhancements are mainly coming from comparison and mapping with OSM
The original request is here etalab/transport-profil-netex-fr#17 (French Accessibility Profile)

First discussed in NeTEx meeting # 16 (June 23)
For information, the tool itself is accessible here during its development phase and will be moved to a national repository once finalised:
https://gitlab.com/yukaimaps/yukaidi for the accessibility capture tool
https://gitlab.com/yukaimaps for the framework (NeTEx implementation, other imports, mobile app, etc.)

More similar PR (on other accessibility related objects) will follow

Aurige and others added 2 commits November 3, 2023 10:21
Enhancements of the Pathlink proposed by the French team working on an accessibility capture tool
These enhancements are mainly coming from comparison and mapping with OSM
The original request is here etalab/transport-profil-netex-fr#17

First discussed in NeTEx meeting # 16 (June 23)
@Aurige
Copy link
Contributor Author

Aurige commented Nov 3, 2023

I targeted the EPIAP branch as it is relevant to accessibility ... the 'next' branch could also be an option.

@skinkie
Copy link
Contributor

skinkie commented Nov 3, 2023

These are actual additions to EPIAP, completely new information not an extra element that already existed in another XSD-group. This is a prime example of something that should have been proposed while we had the working group. Now you are introducing breaking changes yourself, and everything is OK? This feels hypocritical @Aurige considering what you wrote in #533.

I am against pointing to OpenStreetMap wiki URI's, since the information is not stable. In addition, some of these elements are subjective.

@Aurige
Copy link
Contributor Author

Aurige commented Nov 3, 2023

Not breaking anything here ... nothing mandatory, previously validating files will still validate, and that's what is expected with backward compatibility !
We can of course remove the reference to OSM from the XSD (it just it explains where it comes from and OSM is doing a pretty good job)
And @skinkie could you please stay on more respectul comments

@Aurige Aurige added the needs documentation update The NeTEx document needs to be updated label Nov 3, 2023
@skinkie
Copy link
Contributor

skinkie commented Nov 3, 2023

Not breaking anything here ... nothing mandatory

You are introducing new elements in the middle of a PathDescriptionGroup. Any file with this new contents will not work on an older schema, unless the parser explicitly is instructed to ignore those and continue with the rest of the element. The exact opposition you had for the reordering of elements: breaking the paper.

I find it curious that after our conversion yesterday, you are now suggesting changes to EPIAP that should have been discussed within the WG, because they might be valid additions, but were not followed up before by France.

It becomes evident why France is against changes in order, but for introducing extra elements. It seems that developers thought it was a smart idea to hand code the entire NeTEx generation using XML elements and XPath parsing, starting in December 2022, while significantly better ways have been advocated by Entur and Data4PT.

https://gitlab.com/yukaimaps/yukaimaps-netex2wdm/-/blob/main/netex2wdm/converter.py?ref_type=heads
https://gitlab.com/yukaimaps/yukaimaps-wdm2netex/-/blob/main/wdm2netex/converter.py?ref_type=heads

@Aurige
Copy link
Contributor Author

Aurige commented Nov 3, 2023

Backward compatibility is focusing on the fact that files validating with the previous version of the schema need to still validate with the new one. Of course using new feature of the new schema cannot be recognised by the old schema, and that's true for any single extension we have done.
This work in France was done after the end of the EPIAP group's work ... so until we have a time travel machine, I'm afraid that we need to accept that people work after we deliver the XSD and Documents.

I'm not commenting on the rest of your comment which is irrelevant, unkind, and wide of the mark

@skinkie skinkie added the enhancement non semantic enhacement: technical enhancement, etc. label Nov 3, 2023
Copy link
Collaborator

@duexw duexw left a comment

Choose a reason for hiding this comment

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

TactileGuidingStripStatusEnumeration has only values primitive, incorrect, and bad. But there seems to be no value for "good" or "correct"
If you want to conform to OSM, the enum needs another value for "contrasted", which would also make "TactileGuidingStripContrast" unnecessary.
Of course you could not have combinations then like incorrect and contrasted.

@Aurige
Copy link
Contributor Author

Aurige commented Nov 9, 2023

Thanks @duexw ... I agree with you and I'm not sure that incorrect and contrasted is a meaningful use case
I will update accordingly

Merge of TactileGuidingStripContrast and TactileGuidingStripStatus
Addition of goodAndContrasted value to TactileGuidingStripStatusEnumeration
Addition of defaults
@@ -191,7 +191,7 @@ Rail transport, Roads and Road transport
<xsd:documentation>Nature of the tactile warning strips (in the direction of the PATH LINK). +v1.1</xsd:documentation>
</xsd:annotation>
</xsd:element>
<xsd:element name="TactileWarningStripContrast" type="xsd:boolean" minOccurs="0">
<xsd:element name="TactileWarningStripContrast" type="xsd:boolean" minOccurs="0" default="true">
Copy link
Contributor

Choose a reason for hiding this comment

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

So this would mean that there is always contrast?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, it is just to say that the contrast is good by default and you need to be explicit if not... just a proposal here

@ue71603 ue71603 added this to the netex_1.3 milestone Dec 1, 2023
@ue71603
Copy link
Contributor

ue71603 commented Dec 6, 2023

Stefan: For #538 to me it feels like a view on top of the information, while in other sections we communicate precise details, for example review FlooringStatusEnumeration.

@ue71603
Copy link
Contributor

ue71603 commented Dec 19, 2023

@Joostb61 @skinkie @Aurige @duexw @trurlurl updated with example. It seems to me that it is ready.

Copy link
Contributor

@skinkie skinkie left a comment

Choose a reason for hiding this comment

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

I still think this should not go via EPIAP to Master. It is too vague. Next, OK.

@ue71603
Copy link
Contributor

ue71603 commented Dec 21, 2023

@Aurige If for you ok, then I will redo it for the next branch.

@Aurige
Copy link
Contributor Author

Aurige commented Dec 21, 2023

@Aurige If for you ok, then I will redo it for the next branch.

fine for me

@ue71603
Copy link
Contributor

ue71603 commented Dec 22, 2023

addressed in #624

@ue71603 ue71603 closed this Dec 22, 2023
@trurlurl
Copy link
Collaborator

trurlurl commented Jul 4, 2024

Removing "needs documentation update" label since closed without having been merged.

@trurlurl trurlurl removed the needs documentation update The NeTEx document needs to be updated label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement non semantic enhacement: technical enhancement, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants