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

Cross-check the domain/range statements in the vocabulary with the VCDM spec #1319

Closed
iherman opened this issue Oct 18, 2023 · 13 comments
Closed
Assignees
Labels
editorial Purely editorial changes to the specification. pr exists

Comments

@iherman
Copy link
Member

iherman commented Oct 18, 2023

This is a fresh version of #1263; to be done based on the latest version of the vocabulary, diagram, etc.

@iherman
Copy link
Member Author

iherman commented Nov 1, 2023

The issue was discussed in a meeting on 2023-11-01

  • no resolutions were taken
View the transcript

2.2. Cross-check the domain/range statements in the vocabulary with the VCDM spec (issue vc-data-model#1319)

See github issue vc-data-model#1319.

Ivan Herman: Originally people should have reviewed domain and range statements for vocab. There was an issue where these were incorrect. Proposal to close 1319 and if there is a specific issue with a property, raise a separate issue.

@iherman iherman added the pending close Close if no objection within 7 days label Nov 1, 2023
@msporny
Copy link
Member

msporny commented Nov 11, 2023

This is my review of the vocabulary mapping to the VCDM spec:

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#VerifiableCredential

  • Seems to be missing confidenceMethod, proof, name, and description in it's domain.
  • The difference between "Domain of:" and "In the domain of:" is confusing to me. I can't tell what it's trying to express. I believe the first is "Can contain these properties" and the latter is ??? -- I don't know.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#VerifiablePresentation

  • Seems to be proof in it's domain.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#credentialSubject

  • Contains text about the property that should be deleted.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#holder

  • Contains text about the property that should be deleted.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#issuer

  • Contains text about the property that should be deleted.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#relatedResource

  • Contains text about the property that should be deleted.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#verifiableCredential

  • Contains text about the property that should be deleted.

The items above are the only things I found. Thanks for all the work on the vocabulary @iherman ! :)

@msporny msporny added ready for PR This issue is ready for a Pull Request to be created to resolve it and removed pending close Close if no objection within 7 days labels Nov 11, 2023
@iherman
Copy link
Member Author

iherman commented Nov 12, 2023

(When I say "done", it means that I have a branch on my machine, a future PR, where I have made the change.)

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#credentialSubject

  • Contains text about the property that should be deleted.

Done.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#holder

  • Contains text about the property that should be deleted.

You mean the text "The property's value should be a URL, i.e., not a literal." ?

That is an automated text that reflects the following statement in the formal vocabulary:

{
      "@id": "cred:holder",
      "@type": [
        "rdf:Property",
        "owl:ObjectProperty"
      ],
     …

i.e., the owl:ObjectProperty part, and reflects the feature that the value must be URL. I know that is also said in the formal spec (hence the object property statement), but I claim it is better to draw the attention in the HTML as well.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#issuer

  • Contains text about the property that should be deleted.

Done

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#relatedResource

  • Contains text about the property that should be deleted.

Same as the holder case.

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#verifiableCredential

  • Contains text about the property that should be deleted.

You mean the text "The value of this property identifies a Verifiable credential graph. (Informally, it indirectly identifies a Verifiable credential contained in a separate graph.)"?

I was hesitant about this. I know the new version of the spec makes it explicit that the value is a graph but, I believe, draw the attention on this in the HTML version is better. I have removed it temporarily (ie, put it in comment) but maybe others will have a different opinion...

@iherman
Copy link
Member Author

iherman commented Nov 12, 2023

The difference between "Domain of:" and "In the domain of:" is confusing to me. I can't tell what it's trying to express. I believe the first is
"Can contain these properties" and the latter is ??? -- I don't know.

I am all ears for a better terminology, but I am not sure what to use (note that any change will require a change in the yml2vocab script, these are automatically generated).

Looking at validFrom: this is a "shared" property between a VerifiableCredential and a VerifiablePresentation. Formally, this means that the domain of validFrom is the union of the two classes. Hence saying that VerifiableCredential is "in the domain of" validFrom. This is different from, say, the credentialSchema property, whose only domain is VerifiableCredential.

Maybe saying "Included in the domain of" instead of "In the domain of" would work better?

@iherman
Copy link
Member Author

iherman commented Nov 12, 2023

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#VerifiableCredential

  • Seems to be missing confidenceMethod, proof, name, and description in it's domain.

This reflects the fact that these properties do not have a domain statement in the vocabulary (the domain is only defined on properties, these cross references are just generated into the HTML to make it more readable. They do not reflect direct ontology statements).

Taking them on-by-one:

  • confidenceMethod: the definition refers back to a CG Note. That, by itself, may be a problem, but, in any case, I did not find any reference to the domain of this (reserved!) property. I need the experts' statement on whether this, or something else, should be added.
  • proof: first of all, this term is not defined in this vocabulary, and those cross-references are done for the terms in the same vocabulary only (the script is not prepared to seek those extra cross-references across vocabularies). But even beyond that: if I look at the credential vocabulary, the proof property is defined very generally, ie, it is applicable on any RDF resource. I believe it is intentional. Hence, adding a domain statement would be a mistake.
  • name: is not defined by us (but by schema.org), hence, no property statement should be added to the vocabulary (that would generate such an reference).
  • description: same as name

https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#VerifiablePresentation

  • Seems to be proof in it's domain.

See my comment above on proof

@iherman
Copy link
Member Author

iherman commented Nov 15, 2023

The issue was discussed in a meeting on 2023-11-15

  • no resolutions were taken
View the transcript

3.4. Cross-check the domain/range statements in the vocabulary with the VCDM spec (issue vc-data-model#1319)

See github issue vc-data-model#1319.

Manu Sporny: I was able to review Ivan's changes; thank you Ivan.

Ivan Herman: I responded to the issues manu raised.

Brent Zundel: I expect that the results of the conversation will lead to a pull request with changes.


@TallTed
Copy link
Member

TallTed commented Nov 16, 2023

Maybe saying "Included in the domain of" instead of "In the domain of" would work better?

Reversing some statements and/or using different predicates, might help...

all rdfs:domain values must be true (intersection; helpful for inference)

any zero or more dcam:domainIncludes (inspired by schema:domainIncludes) might be true (union; open world!)

all rdfs:range values must be true (intersection; helpful for inference)

any zero or more dcam:rangeIncludes (inspired by schema:rangeIncludes) might be true (union; open world!)

@iherman
Copy link
Member Author

iherman commented Nov 17, 2023

I am not sure how to interpret your comment in #1319 (comment), @TallTed.

We are talking about the (generated) English sentence "In the domain of" in, say, https://w3c.github.io/vc-data-model/vocab/credentials/v2/vocabulary.html#VerifiablePresentation, like below:

Screenshot 2023-11-17 at 10 01 56

More exactly, we are looking at a way to replace it. The TTL/JSON-LD vocabulary is not in question (it uses a perfectly valid OWL construction for the union of possible domains). My question was whether, say, "Part of the domain of" would be a suitable replacement.

The two lines in the example above are solely an information for the reader. There is no corresponding RDF sentence in the term definition of VerifiablePresentation.

@msporny
Copy link
Member

msporny commented Nov 18, 2023

I've always preferred simpler language, so:

  • instead of "Domain of" and "In the domain of", we can just combine the two and say: "Associated properties:"
  • instead of "Range", we can say: "Associated classes:"

I know that's not accurate, but I doubt 95%+ of the developers that might find themselves reading the vocabulary document will know what the difference is (or the difference doesn't matter to them).

@iherman
Copy link
Member Author

iherman commented Nov 19, 2023

I've always preferred simpler language, so:

  • instead of "Domain of" and "In the domain of", we can just combine the two and say: "Associated properties:"
  • instead of "Range", we can say: "Associated classes:"

I know that's not accurate, but I doubt 95%+ of the developers that might find themselves reading the vocabulary document will know what the difference is (or the difference doesn't matter to them).

I would be o.k. with this, but before I modify the underlying script (because that is what it takes) I would need more positive reactions (this is a purely editorial choice, so we do not need a formal WG resolution, but nevertheless...)

@msporny
Copy link
Member

msporny commented Nov 26, 2023

I'm satisfied that @iherman has responded to all of the structural review comments I had.

The only changes remaining are to address the "Contains text about the property that should be deleted." statements here: #1319 (comment)

Since these are all editorial changes, I'm relabeling this issue to post CR.

@msporny msporny added editorial Purely editorial changes to the specification. post-CR and removed before CR labels Nov 26, 2023
@iherman iherman added pr exists and removed ready for PR This issue is ready for a Pull Request to be created to resolve it labels Nov 26, 2023
@iherman
Copy link
Member Author

iherman commented Nov 26, 2023

I have created #1361 to cover the minor changes in the vocabulary.

I have not changed the items discussed in #1319 (comment); that is subject to a change in the script and not in this repo. I would actually suggest to review these types of styling issues when the dust settles around both the VCDM and the DI vocabularies (a possible change would affect both vocabularies, and possible other vocabularies that may come to the fore).

@msporny
Copy link
Member

msporny commented Dec 7, 2023

PR #1361 has been merged, addressing the remaining issues in this PR; closing.

@msporny msporny closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Purely editorial changes to the specification. pr exists
Projects
None yet
Development

No branches or pull requests

3 participants