-
Notifications
You must be signed in to change notification settings - Fork 5
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
FC: Collective multiplicities not specified correctly #140
Comments
I agree but... Either way is ok but I usually prefer to make a tighter model instead of adding more validation checks. |
I get that you aren't advocating for this, but for those that are, note that the collection is only expected to satisfy these constraints after editing / production is completed. To include these items in a collection and then set their lower multiplicity to zero doesn't make much sense; why are they in a collection at all? |
I think the collections should have a min of 1 member. I was paraphrasing the arguments for why they got changed to 0, which did not come from me. I don't remember if it was another Github issue or an email thread where that discussion happened. |
I'm probably the source of that :-) I noticed the vast preponderance of mandatory associations and asked for them to be taken out so they match what is required from the DCEG. My understanding, having spoken to Jeff at TSM (and before) was that the conditional multiplicity wasn't mandatory and the only mandatory associations were things like IslandGroup and TextPlacement where they make no sense to exist on their own. However, I agree that where an association is mandated by S-58 and we can replicate it in the S-101 FC we should probably try and do so. But, this could create a large overhead for producers if the associations either don't exist or aren't convertible in the source S-57 (but I would have thought they should be?). There's a riusk though that we introduce 100s of validation errors in test data. I was at the IC-ENC conference with Jeff this week and he showed me the revisions to the associations he's done for the DCEG and I'm not sure the mandatory/optional question is answered yet, but I think we're starting from the point where most are still optional. Maybe we should go through those mandated from S-58 and try to come up with a definitive list. Now the naming convention is agreed upon it would be good to do this before June PT??? |
previous issue on multiplicities is here: |
@DavidGrant-NIWC @kusala9 @TDYCARHugh @JeffWootton Currently, all Collective Multiplicity have been removed from the DCEG. |
The problem originates in the DCEG, you would need @JeffWootton to update the tables. See https://iho.int/en/s-100wg6-2022 paper and presentation 4.26 When a binding has multiple targets, the multiplicity applies to the collection. So, the following binding indicates that a The FC is accurately modeling the information from the DCEG (which is wrong): The proper way to describe this relationship is via separate bindings:
or alternatively:
|
Collective multiplicities are not specified correctly. For instance,
FairwaySystem
has anAidsToNavigationAssociation
which uses a collective multiplicity (there are multiple feature types in the binding) and the lower bound of the collective multiplicity is zero (indicating that there doesn’t have to be an aid associated with theFairwaySystem
):The DCEG shows this as requiring that one of the possible features must be present (the lower bound of the collective multiplicity is always one, indicating that an aid must be associated with the
FairwaySystem
):This issue seems to be present for every binding which uses a collective multiplicity.
The text was updated successfully, but these errors were encountered: