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

Issue 230: Update description of sh:pattern behavior with IRI nodes #231

Draft
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from

Conversation

ajnelson-nist
Copy link

This patch series will address update test results, and possibly other documentation, around sh:pattern and how it behaves against IRI nodes. E.g., there will be clarification on what should happen with use cases like this:

@prefix ex: <http://example.org/> .

ex:Shape-1
    a sh:NodeShape ;
    sh:pattern "^https" ;
    sh:targetNode ex:Node-1 ;
    .

I.e., should the sh:sourceConstraintComponent be sh:PatternConstraintComponent for violating the pattern (not starting with https)? sh:NodeKindConstraint for being an IRI node, even though sh:nodeKind is not used in this shape?

The intent of this PR is to at least close #230 , and possibly generate editor's draft revisions to cover use case 1 in #228 .

I suspect the working group needs to confirm whether this will be a backwards-compatible change.

I could also see SHACL-SHACL being updated to further constrain where sh:pattern can be used.

Aside: This is also my first PR against this repo, so rules-of-the-road pointers would be appreciated, such as appropriate target branch.
ACKs are due to @afs, @tfrancart, and @tpluscode from conversation on #228 ; what's the appropriate git log tag to use? Cc:?
I'll force-push the first patch for the benefit of git log and git blame after guidance on ACKs.

Per discussion on Issues 228 and 230.

References:
* #228
* #230

Signed-off-by: Alex Nelson <[email protected]>
@ajnelson-nist ajnelson-nist linked an issue Feb 5, 2025 that may be closed by this pull request
@@ -40,7 +40,7 @@ ex:TestShape
rdf:type sh:ValidationResult ;
sh:focusNode ex:Test ;
sh:resultSeverity sh:Violation ;
sh:sourceConstraintComponent sh:PatternConstraintComponent ;
sh:sourceConstraintComponent sh:NodeKindConstraintComponent ;

Choose a reason for hiding this comment

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

The original was right, this is wrong

Copy link
Author

Choose a reason for hiding this comment

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

I have no objection with rolling this back, but I'd like the reasoning to be unambiguous from updates to the SHACL-Core document.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think should be sh:PatternConstraintComponent (which jena-shacl passes).

Even if it were a blank node - so some kind of argument error - it is sh:sourceConstraintComponent which is the constraint failing, not some kind of an error class.

@afs
Copy link
Contributor

afs commented Feb 5, 2025

I'll force-push the first patch for the benefit of git log and git blame after guidance on ACKs.

We-all need to agree on process. We have one repo for many documents. Force-push may loose work on other documents.

The editor-drafts-1.2 branch was created recently (Jan 6) by @HolgerKnublauch so I though it was his PR branch.

We need a protected branch for the documents.

@ajnelson-nist
Copy link
Author

I'll force-push the first patch for the benefit of git log and git blame after guidance on ACKs.

We-all need to agree on process. We have one repo for many documents. Force-push may loose work on other documents.

The editor-drafts-1.2 branch was created recently (Jan 6) by @HolgerKnublauch so I though it was his PR branch.

We need a protected branch for the documents.

Apologies, I never had intention of force-pushing the editor-drafts-1.2 branch. My comments here were only meant to be about revising the first commit of this patch series, 1064859, so its log-message reflected contributors accurately.

That effort might be moot if the patch'll be superseded, though.

@simonstey
Copy link
Contributor

simonstey commented Feb 6, 2025

I'll force-push the first patch for the benefit of git log and git blame after guidance on ACKs.

We-all need to agree on process. We have one repo for many documents. Force-push may loose work on other documents.

The editor-drafts-1.2 branch was created recently (Jan 6) by @HolgerKnublauch so I though it was his PR branch.

We need a protected branch for the documents.

+1, besides a protected main/dev branch, what about having dedicated branches for each of the individual documents? e.g., one for shacl-core, shacl-sparql, shacl-inference, shacl-cs, ... ?

@gkellogg How was this handled in the JSON-LD WG with its documents on framing etc.?

edit: JSON-LD had multiple repos https://github.com/search?q=topic%3Ajson-ld-wg+org%3Aw3c&type=Repositories

Base automatically changed from editor-drafts-1.2 to gh-pages February 6, 2025 06:48
@tpluscode
Copy link

dedicated branches for each of the individual documents

sounds like overkill

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sh:pattern does not allow testing IRIs
5 participants