iho-ohi / S-101-Documentation-and-FC

Repository issues of S-101 document and feature catalogue
22 stars 5 forks source link

Review of Draft 1.3.0 Feature Catalogue #151

Open TomRichardson6 opened 3 months ago

TomRichardson6 commented 3 months ago

A draft 1.3.0 Feature Catalogue has been posted here, comments are invited on this document. Please comment here with any issues that you identify.

S-101-Documentation-and-FC/S-101FC/FeatureCatalogue.xml at main · iho-ohi/S-101-Documentation-and-FC (github.com)

DavidGrant-NIWC commented 3 months ago

The FC references the staging server (because that’s the only place the 5.2 schema is available). The 5.2 schemas need to be moved to the production server and then the FC should be updated accordingly.

image

TomRichardson6 commented 3 months ago

Some initial observations on this draft, review not yet completed.

@DavidGrant-NIWC agree that this will need to be amended in the final 1.3.0 Catalogue.

XML line 8527 DCEG ref 27.125

Change - Corrected the definition of attribute maximum permitted vessel length to refer to length rather than draught.

Observation - Definition continues to refer to draught should be amended to length.

image

XML line 8554 DCEG ref 28.10

Change - Amended attribute type for attribute measurement distance maximum from type Real to type Integer.

Observation - Value type is still listed as Real should be Integer

image

XML line 8569 DCEG ref 28.11

Change - Amended attribute type for attribute measurement distance minimum from type Real to type Integer.

Observation - Value type is still listed as Real should be Integer

image

DavidGrant-NIWC commented 3 months ago

144 and #145 are uncorrected.

TDYCARHugh commented 2 months ago

Agree with David's comment that textPattern fields are not currently valid Regex.

Feature binding from features to TextPlacement should have an association roleType of 'association' from TextPlacement to feature should be roleType = 'composition'. This was correct in 1.2 but got reversed in 1.3 draft.

`

0 1 ` should be `` UpdatedInformation association. From feature to UpdateInformation with role 'theUpdate' should be 0 to many as in DCEG. ` 0 1 ` should be: `` Inconsistencies with DCEG: names of roles are not exactly matching for example ‘theUpdate’ versus the The Updated Object. It would be nice to actually have the proper camel case codes for features, attributes, roles etc. listed in the DCEG for consistency with the FC. Also noticed a DCEG error: Under CautionArea the CautionAreaAssociation association to Archipelagic Sea Lane should be using the role 'theComponent' and multiplicty of 0..*.
TomRichardson6 commented 2 months ago

Two further comments

DCEG ref 27.184

The definitions for rectangle (horizontal) and rectangle (vertical) have not been updated to be consistent with the DCEG.

Untitled Rectangle (Horizontal) Where the two longer opposite sides are standing horizontally. A rectangle is a plane figure with four right angles and four straight sides, opposite sides being parallel and equal in length. A horizontal rectangle is where the two longer opposite sides are standing horizontally. 20 1143 Rectangle (Vertical) A rectangle is a plane figure with four right angles and four straight sides, opposite sides being parallel and equal in length. A vertical rectangle is where the two longer opposite sides are standing vertically. 21 1144 DCEG ref 27.192 Linked to the other comment on Regex for textPattern the change of value of sounding from 0.01 to 0.1 has not been reflected in the FC. Propose that a broader review and update of all textPattern values is needed for FC 2.0.0.
TDYCARHugh commented 2 months ago

I think that the roles for TextPlacement TextAssociation might be backwards. Currently it is defining that the TextPlacement feature is referred to as 'theCartographicText' and the feature for which the text is being placed is being referred to as 'thePositionProvider'. It seems to read better if these were swapped in the bindings each way. I would have preferred simpler roles like 'thePlacement' and 'theFeature'.

TomRichardson6 commented 2 months ago

In addition see issue which covers missing associations for StructureOverNavigableWater

https://github.com/iho-ohi/S-101-Documentation-and-FC/issues/156

gorogara commented 2 months ago

@TomRichardson6 Updated List

  1. Maximum Permitted Vessel Length https://github.com/bluemap-github/S-101-Documentation-and-FC/commit/0c85ae8ed19ed5cd71be073153ebdf57b23d44e3

  2. Measurement Distance Maximum(&Minimum) https://github.com/bluemap-github/S-101-Documentation-and-FC/commit/6d3278c2600db9dbc1a0b9657e5aa4b75afe9af3

  3. Rectancle (Horizontal & Vertical) https://github.com/bluemap-github/S-101-Documentation-and-FC/commit/5353489056982aca1bd5a561a1cd6ac08917d266

  4. Light Sectored LightSectored had duplicate Associations. I have deleted the unnecessary one. https://github.com/bluemap-github/S-101-Documentation-and-FC/commit/e7ed0cb5778fcd907a416a7748bf9e48484daa86

gorogara commented 2 months ago

@TomRichardson6 Questions List

  1. Precision of value of sounding If the precision of the value of sounding is 0.1, should it be entered as <S100CD:precision>1</S100CD:precision> in the FC? And would <S100CD:precision>2</S100CD:precision> correspond to 0.01.

  2. Precision of other attributes It seems that the precision of other attributes can be updated by retrieving the registered values from the GI Registry. Should the updates be made using the values from the GI Registry?

  3. Roles of TextAssociation @TDYCARHugh @TomRichardson6 Should I reflect the opinions about roles of TextAssociation.

  4. StructureOverNavigableWater StructureOverNavigableWater There are other Feature Associations on the StructureOverNavigableWater page, but they are not included in the Association table in Chapter 25. I create everything based on Chapter 25, so this issue arose. Will Chapter 25 be updated according to #156?

rmalyankar commented 2 months ago

@TomRichardson6 Questions List

  1. Precision of value of sounding If the precision of the value of sounding is 0.1, should it be entered as <S100CD:precision>1</S100CD:precision> in the FC? And would <S100CD:precision>2</S100CD:precision> correspond to 0.01.

I am not Tom, but yes, precision 0.1 and 0.01 would be entered in an FC as 1 and 2 respectively. Note that the type of precision in S-100 is "non-negative integer".

  1. Precision of other attributes It seems that the precision of other attributes can be updated by retrieving the registered values from the GI Registry. Should the updates be made using the values from the GI Registry?

This may be more complicated. I think the registry may be missing entries for precision or have the wrong format ("0.01") in some cases. In case of conflict between the registry and the DCEG, the DCEG should prevail.

DavidGrant-NIWC commented 2 months ago

The correct value to use for precision is not well defined. See https://github.com/iho-ohi/S-101-Documentation-and-FC/issues/104#issuecomment-2016046518 image

TDYCARHugh commented 2 months ago

TextPlacement binding with role theCartographicText should be roleType association, association from TextPlacement to feature is roletype composition.

TomRichardson6 commented 1 month ago

@gorogara

Responses on your questions please apply these but no need to prepare a further version at this time.

  1. Please use 1 and I will take an action to explore this further and confirm. This will eventually need clarity in S-100.

  2. Yes in future but no action at this stage we need to confirm the approach to precision. As Raphael says the Registry needs checking as well.

  3. Yes please apply this change as per Hugh's comment.

  4. Please retain this as consistent with the DCEG until a further version of the DCEG is prepared.