w3c / rdf-star

RDF-star specification
https://w3c.github.io/rdf-star/
Other
120 stars 23 forks source link

suggest metadata in manifest turtle file #198

Closed gatemezing closed 2 years ago

gatemezing commented 3 years ago

This a based on the suggestion of the issue #197.

gkellogg commented 3 years ago

CI fails due to syntax errors in some of the manifest files. Basically, you need to be able to run tests/make-jsonld-manifests.rb and tests/make-heml-manifests.py successfully, but it should work with valid Turtle inputs. There may be more required to properly reflect added properties in JSON-LD and HTML views.

pchampin commented 3 years ago

This was discussed during today's call: https://w3c.github.io/rdf-star/Minutes/2021-09-03.html#t03

afs commented 3 years ago

I took a diff from the current state of this PR and applied it to a local up-to-date repo.

The changes for sparq/, trig/, turtle/ *.ttl parse and the tests run.

  1. The semantics/manifest did not merge - possibly the PR is based off an older version?
  2. There don't seem to be changes to sparql/eval.
  3. Alignment issues.
  4. Development comments.
  5. Additional trailing whitespace has been added

semantic/manifest.ttl

"git apply semantics.diff" gives:

semantics.diff:17: trailing whitespace.
PREFIX rdf:    <http://www.w3.org/1999/02/22-rdf-syntax-ns#> 
semantics.diff:18: trailing whitespace.
PREFIX rdfs:   <http://www.w3.org/2000/01/rdf-schema#> 
semantics.diff:19: trailing whitespace.
PREFIX mf:     <http://www.w3.org/2001/sw/DataAccess/tests/test-manifest#> 
semantics.diff:20: trailing whitespace.
PREFIX rdft:   <http://www.w3.org/ns/rdftest#> 
semantics.diff:21: trailing whitespace.
PREFIX trs:    <https://w3c.github.io/rdf-star/tests#> 
error: patch failed: tests/semantics/manifest.ttl:1
error: tests/semantics/manifest.ttl: patch does not apply

Alignment: the problem show when the files are viewed including in this PR.

Tabs width are an unknown (2? 4? 8? or "smart tabs" in an IDE) - it is better to use tabs/spaces consistently and the files have spaces.

On linux, the expand command will do most of the work.

This is what I see in sparql-syntax/manifest.ttl with tab width of 8:

trs:manifest  rdf:type mf:Manifest ;
    rdfs:label "SPARQL-star Syntax Tests"@en ;
    rdfs:label "La suite des tests pour SPARQL-star"@fr;
        rdfs:label "Conjunto de pruebas para SPARQL-star"@es;
        dct:issued "2021-06-21"^^xsd:date ; #not sure if this was the first release date
    rdfs:seeAlso <https://w3c.github.io/rdf-tests/> ;
        dct:modified "2021-07-18"^^xsd:date ; # change to the exact date
        dct:licence <https://www.w3.org/Consortium/Legal/2008/03-bsd-license> ;
        dct:creator [ foaf:homepage <https://w3c.github.io/rdf-star/> ; foaf:name " W3C RDF-star Working Group" ] ;
    mf:entries

In trig/eval/manifest (which for some reason has an indent of 3 and was probably supposed to be 4 - ditto turtle/eval):

trs:manifest  rdf:type mf:Manifest ;
   rdfs:label "TriG-star Evaluation Tests"@en ;
   rdfs:label "La suite des tests pour TriG-star"@fr;
   rdfs:label "Conjunto de pruebas para Trig-star"@es;
    dct:issued "2021-06-21"^^xsd:date ; #not sure if this was the first release date
   rdfs:seeAlso <https://w3c.github.io/rdf-tests/> ;
    dct:modified "2021-07-18"^^xsd:date ; # change to the exact date
    dct:licence <https://www.w3.org/Consortium/Legal/2008/03-bsd-license> ;
    dct:creator [ foaf:homepage <https://w3c.github.io/rdf-star/> ; foaf:name " W3C RDF-star Working Group" ] ;
    mf:entries

Development comments

These appear in several files:
#not sure if this was the first release date
# change to the exact date

These shouldn't in the merge.

Trailing whitespace

An example taken from from tests/semantics/manifest.ttl but it seems to be in all files: when PREFIX is added there a trailing space on URIs:

+PREFIX rdf:    <http://www.w3.org/1999/02/22-rdf-syntax-ns#>_

where _ is an added space. This happens on added and existing prefixes so it ials changes to lines not otherwise changed.

gatemezing commented 3 years ago

I took a diff from the current state of this PR and applied it to a local up-to-date repo.

Thanks for your time Andy! The changes for sparq/, trig/, turtle/ *.ttl parse and the tests run.

  1. The semantics/manifest did not merge - possibly the PR is based off an older version?
  2. There don't seem to be changes to sparql/eval.
  3. Alignment issues.
  4. Development comments.
  5. Additional trailing whitespace has been added

I'll try to address the points in my next commits. Thanks.

gatemezing commented 3 years ago

@afs, any hint on this conflict issue with the turtle/eval/manifest file? TIA

afs commented 3 years ago

Whitespace difference.

TallTed commented 3 years ago

@gatemezing -- fwiw, I often find it's easier to do the resolve-conflicts thing by copying the conflicting section into an external editor which can display "invisible characters" like tabs and spaces (I use BBEdit on macOS), splitting it into two local docs, and running local diffs to reconcile them (which sometimes means changes flow both ways). When they're identical, I copy and paste one back to Github.

gatemezing commented 3 years ago

@gatemezing -- fwiw, I often find it's easier to do the resolve-conflicts thing by copying the conflicting section into an external editor which can display "invisible characters" like tabs and spaces (I use BBEdit on macOS), splitting it into two local docs, and running local diffs to reconcile them (which sometimes means changes flow both ways). When they're identical, I copy and paste one back to Github.

Thanks @TallTed ! I'm struggling in Windows with Notepad++.

gatemezing commented 3 years ago

Whitespace difference.

Thanks for the hint! Next, it's to make ruby works to validate the isomorphism file. Some hours of testing ahead :)

gatemezing commented 3 years ago

CI fails due to syntax errors in some of the manifest files. Basically, you need to be able to run tests/make-jsonld-manifests.rb and tests/make-heml-manifests.py successfully, but it should work with valid Turtle inputs. There may be more required to properly reflect added properties in JSON-LD and HTML views.

@gkellogg - Yeah, I'm trying to make the tests using make-jsonld-manifests.rb. Is there a way to use ruby to convert the .ttl file to jsonld? BTW, I just realized that when validating the tests/manifest-context.jsonld using online playground I get an error jsonld.SyntaxError: Invalid JSON-LD syntax; "@id" value must a string. Is that normal?

gkellogg commented 3 years ago

I’ll take a look at it and suggest changes. It’s likely that the json-led frame is not handling multi-language literals (which will also affect the html generation).

gatemezing commented 3 years ago

I’ll take a look at it and suggest changes. It’s likely that the json-led frame is not handling multi-language literals (which will also affect the html generation).

Thanks! I made some changes in the manifest-context.jsold file to take into account those multi-lingual labels. I can't just figure out from the errors output of the .rb script, where exactly to make the changes. If you find the fix for one file, I can take care to make rest of the changes for the remaining files.

gkellogg commented 3 years ago

The reason for the failure of the semantics manifest is that the "test" prefix was removed. This would be the only manifest that still uses it (for test:approval). It happens that the JSON-LD framing depends on this being there to be able to show manifests that are not included in mf:entries. This also creates a parsing error, as the "test" prefix is used without being defined. So, the simplest thing would be to re-introduce the prefix definition:

PREFIX test:   <http://www.w3.org/2001/sw/DataAccess/tests/test-dawg#>

Alternatively:

Do that, and make-jsonld-manifests.rb completes satisfactorily. The HTML manifests also build, but I didn't check them to be sure they display all the pertinent information.

gatemezing commented 3 years ago

So, the simplest thing would be to re-introduce the prefix definition:

Hi @gkellogg, this solved the errors. I've not checked the html output. Thanks again!

afs commented 3 years ago

This needs some tidy up.

  1. It looks like it is based off the repo as of about 9 weeks ago.
  2. There are a lot of intermediate commits (about 17).
  3. There are conflicts on tests/turtle/eval/manifest.ttl, tests/trig/eval/manifest.ttl.

Either

A. Squash the PR to one commit first, pull current main to main in your local repo and then rebase on the PR - only one set of conflicts to handle otherwise there will be conflict after conflict as each individual commit are merged.

Or

B. It might be better to take a diff of the changes, start with an up-to-date fork of the main branch, apply the diff (fixing any problems) and making a new PR or force pushing onto this PR.

gatemezing commented 3 years ago

This needs some tidy up.

  1. It looks like it is based off the repo as of about 9 weeks ago.
  2. There are a lot of intermediate commits (about 17).
  3. There are conflicts on tests/turtle/eval/manifest.ttl, tests/trig/eval/manifest.ttl.

Either

A. Squash the PR to one commit first, pull current main to main in your local repo and then rebase on the PR - only one set of conflicts to handle otherwise there will be conflict after conflict as each individual commit are merged.

Or

B. It might be better to take a diff of the changes, start with an up-to-date fork of the main branch, apply the diff (fixing any problems) and making a new PR or force pushing onto this PR.

Thanks for the hints! I started with option A and sent yet-another-PR. I did not see any conflicts with the main branch. Maybe I'm missing something here. Ufff, quite challenging to make a normal PR in this repo :)

afs commented 3 years ago

Running the manifest.ttl files, I get suite names in a mix of English, French and Spanish. The code looks for one rdfs:label.

Suggestion: Use rdfs:label with non-lang-tag text. Use skos for the language tagged literals.

gatemezing commented 3 years ago

Running the manifest.ttl files, I get suite names in a mix of English, French and Spanish. The code looks for one rdfs:label.

Suggestion: Use rdfs:label with non-lang-tag text. Use skos for the language tagged literals.

Thanks for the suggestions. However, I'm afraid to enter in a new loop with the changes you suggested, that is update both .ttl and .jsonld files, etc. Could the editors safely do that once the PR is merged?

pchampin commented 2 years ago

Unless I'm mistaken, this PR is deprecated by #205. Shall we close this one, then?

afs commented 2 years ago

Closed : replaced by #205.