duraspace / pcdm

Portland Common Data Model
http://pcdm.org/models
Apache License 2.0
90 stars 11 forks source link

Removes pcdm:hasRelatedFile from `context.json` #45

Closed no-reply closed 8 years ago

no-reply commented 8 years ago

pcdm:hasRelatedFile was removed from the RDF(S) in https://github.com/duraspace/pcdm/pull/31, but hung around in the JSON-LD context.

Removing it from there, now.

The second commit (8eb09c9) removes unused namespaces and cleans up trailing whitespace.

ruebot commented 8 years ago

:+1:

awoods commented 8 years ago

Shouldn't "relatedFileOf" also go away? https://github.com/duraspace/pcdm/blob/remove-has-related-file/context.json#L27

no-reply commented 8 years ago

Thanks @awoods.

I removed the inverse relation and squashed the commits.

escowles commented 8 years ago

As long as we're tidying up, should we be declaring namespaces that are not used in the context? Is it standard practice to include namespaces that you expect to be generally useful?

AFAICT, ldp, and rdf aren't actually used in the context (though they are used in the ontology of course). There are also several namespaces that I don't think we're using anywhere: ebucore, foaf, gen, premis, and sweetjpl.

no-reply commented 8 years ago

@escowles I cleaned up the namespaces that aren't used. ldp: does appear in a couple of places. Otherwise, I think your list was correct.

Do we need some standard way of testing this context? It would help my confidence in making these changes if we had clear requirements that, ideally, could be checked automatically.

escowles commented 8 years ago

@no-reply -- my bad, I didn't expand the file all the way to the bottom so the LDP usage was still hidden.

I agree it would be great to automatically test the ontology files. We could minimally test the RDF/XML by transforming it with the rdfs2html.xsl stylesheet. Similarly, I'm sure we can find tools to validate the JSON is well-formed. But neither of these would catch unused namespaces.

azaroth42 commented 8 years ago

:+1: To test the context, you could write a little script that took known data, expanded and compacted it, then tested to see if it was the same as the input?

awoods commented 8 years ago

This PR looks good. Shall we move it forward while also creating a new issue for automating testing of the context?

escowles commented 8 years ago

+1 to merging this and handling automatic testing in a separate ticket.

no-reply commented 8 years ago

Yes. Though I think we are waiting on an additional :+1:, and whatever else is required by #44.

no-reply commented 8 years ago

(I guess not the additional :+1: anymore, due to @escowles).

ruebot commented 8 years ago

:+1: to merging this and handling automatic testing in a separate ticket.

awoods commented 8 years ago

...now we wait for 7 days :( ..or 7 days from: https://github.com/duraspace/pcdm/pull/45#issuecomment-186705081 which was 2 days ago. Merge target: Feb 26th

azaroth42 commented 8 years ago

In automated testing issue, can we also discuss automated merge/deploy? Scan the comment threads of PRs for sufficient +1s and no -1s, look at commit dates ... seems like an easy algorithm

no-reply commented 8 years ago

This one should be ready, per https://github.com/duraspace/pcdm/pull/45#issuecomment-187310563