uncefact / spec-jsonld

Exposing the UN/CEFACT vocabulary as web semantics
https://service.unece.org/trade/uncefact/vocabulary/uncefact/
13 stars 5 forks source link

specifiedIdentity domain #63

Closed VladimirAlexiev closed 2 years ago

VladimirAlexiev commented 2 years ago

specifiedIdentity is the prop that relates an agent (Party, FinancialInstitution, Person) to Identity. Identity holds a bunch of specific identifiers such as driversLicense, BIC, BEI, passportId, etc.

However, there's also a wrong relation from Identity to Identity:

nissimsan commented 2 years ago

image

nissimsan commented 2 years ago

Make sure this is a transform problem, not a source data problem.

Fak3 commented 2 years ago

I suspect that our class grouping rules are wrong with grouping Financial and Personal Identities into one Identity. We discussed with @kshychko and she can try to change the grouping rules so that we can analyze if it will work better.

kshychko commented 2 years ago

@Fak3, I created the md file with classes that are grouped at the moment, but probably should be split. https://gist.github.com/kshychko/dbcdb3cb6c383622a71f8c093298575a Shared properties are marked as bold. It also revealed the issue with the property naming for some properties like associatedDocument in LineTradeTransaction class, it is listed there three times, but I'll raise a separate ticket on this. From what I can see most of the classes we grouped should be provided as separate classes as they are not that identical. At the moment we have 136 classes in BSP vocabulary, if we split the classes that share the common object class term, we'll get 218 classes. Personally, I think that's OK because that won't produce new properties, that are making the most of the vocabulary content. I suggest we discuss it on the next call.

Fak3 commented 2 years ago

@kshychko in your table there is Referenced_SupplyChainConsignment and SupplyChainConsignment - those should be merged into one, and the same for ReferencedSupplyChainConsignmentItem, and possibly other Referenced classes see https://gist.github.com/kshychko/dbcdb3cb6c383622a71f8c093298575a#consignment

Fak3 commented 2 years ago

CreditorFinancialAccount | DebtorFinancialAccount | FinancingFinancialAccount also look very similar, it is worth merging them, replacing creditorFinancialAccountTypeCode anf debtorFinancialAccountTypeCode with a single typeCode see https://gist.github.com/kshychko/dbcdb3cb6c383622a71f8c093298575a#financialaccount

Fak3 commented 2 years ago

Also GeographicalCoordinate and SpecifiedGeographicalCoordinate - must be merged

Fak3 commented 2 years ago

There are even more contrived cases - ControlSettingParameter and OperationalParameter seems to be the same concept, so worth to merge, but the DocumentContextParameter looks completely separate thing see https://gist.github.com/kshychko/dbcdb3cb6c383622a71f8c093298575a#parameter

kshychko commented 2 years ago

@kshychko in your table there is Referenced_SupplyChainConsignment and SupplyChainConsignment - those should be merged into one, and the same for ReferencedSupplyChainConsignmentItem, and possibly other Referenced classes

Yep, those ones are initially the reason why we wanted to merge the classes. But in this case, the Consignment class has a property of the type Consignment. Is that dependency OK?

nissimsan commented 2 years ago

Decission on July 14 2022: Only take out "Referenced", keep all other qualifiers as separate classes.

Fak3 commented 2 years ago

Perhaps instead of merging classes we can link separate classes together with unece:relatedClass property? It is optional, but may be useful for someone who tries to deduplicate them in the future? I.e:

{
"@id": "unece:CreditorFinancialAccount" ,
"@type": "rdfs:Class",
"unece:relatedClass": ["unece:DebtorFinancialAccount", "unece:FinancingFinancialAccount"]
}
nissimsan commented 2 years ago

@Fak3, Good idea. We'll merge what we have now and this can be implemented later. I would question if this relatedClasses will be used much. I'd prefer to wait for actual requirements for it. Let's not make this a high priority,

nissimsan commented 2 years ago

We've done this - closing.