search

oborel / obo-relations

RO is an ontology of relations for use with biological ontologies
http://oborel.github.io/
Other
91 stars 46 forks source link

Change dc:creator to dcterms:contributor #731

Closed wdduncan closed 1 year ago

wdduncan commented 1 year ago

@matentzn @cthoyt @anitacaron the qc is failing b/c of non-conforming values for the created_by annotation.
E.g., here is a list produced after running make test.

FAIL Rule ../sparql/term-editor-uri-violation.sparql: 33 violation(s)
subject,orcid
http://purl.obolibrary.org/obo/RO_0002207,has developmental precursor
http://purl.obolibrary.org/obo/valid_for_go_gp2term,pg
http://purl.obolibrary.org/obo/valid_for_go_annotation_extension,pg
http://purl.obolibrary.org/obo/RO_0002020,dos
http://purl.obolibrary.org/obo/RO_0012002,pg
http://purl.obolibrary.org/obo/RO_0002021,dos
http://purl.obolibrary.org/obo/RO_0012012,pg
http://purl.obolibrary.org/obo/RO_0002023,dos
http://purl.obolibrary.org/obo/RO_0002019,dos
http://purl.obolibrary.org/obo/RO_0012004,pg
http://purl.obolibrary.org/obo/RO_0002026,dos
http://purl.obolibrary.org/obo/RO_0012001,pg
http://purl.obolibrary.org/obo/RO_0012005,pg
http://purl.obolibrary.org/obo/RO_0002029,dos
http://purl.obolibrary.org/obo/RO_0002017,dos
http://purl.obolibrary.org/obo/RO_0002015,dos
http://purl.obolibrary.org/obo/RO_0012006,pg
http://purl.obolibrary.org/obo/RO_0002018,dos
http://purl.obolibrary.org/obo/RO_0012003,pg
http://purl.obolibrary.org/obo/RO_0002022,dos
http://purl.obolibrary.org/obo/valid_for_go_ontology,pg
http://purl.obolibrary.org/obo/RO_0012007,pg
http://purl.obolibrary.org/obo/RO_0012008,pg
http://purl.obolibrary.org/obo/RO_0002024,dos
http://purl.obolibrary.org/obo/RO_0002025,dos
http://purl.obolibrary.org/obo/RO_0002013,dos
http://purl.obolibrary.org/obo/RO_0012000,pg
http://purl.obolibrary.org/obo/RO_0012009,pg
http://purl.obolibrary.org/obo/RO_0002014,dos
http://purl.obolibrary.org/obo/RO_0012010,pg
http://purl.obolibrary.org/obo/valid_for_gocam,pg
http://purl.obolibrary.org/obo/RO_0012011,pg
http://purl.obolibrary.org/obo/RO_0002016,dos
wdduncan commented 1 year ago

@balhoff when I changed the annotation property I didn't specify the datatype. I assumed the datatype would carry over to the new annotation. My mistake :(

This PR originated from my oversight of merging NTR: has roost prior to the qc checks finishing :(

gouttegd commented 1 year ago

@wdduncan Mind if I commit directly to this PR to fix the remaining QC issues?

wdduncan commented 1 year ago

@gouttegd I don't mind at all!
Thanks!

gouttegd commented 1 year ago

Done and the CI checks now pass.

cthoyt commented 1 year ago

Please please please do not merge this until the commit history on the main branch is fixed

wdduncan commented 1 year ago

What needs to be fixed? I'm not following.

gouttegd commented 1 year ago

@cthoyt The point of this PR is precisely to fix the breakage that was introduced by the merging of #672.

I agree that a hard reset to cancel #672 would be cleaner, but not now that the merge has occurred on the public master branch. Hard resets are good on a local repository to cancel commits that have never been published, but once they have been pushed on a public repo, it’s too late. Rewriting history on a public branch is a bad idea.

cthoyt commented 1 year ago

please see https://github.com/oborel/obo-relations/issues/732. @gouttegd either way, can you help remove all of the spurious diff in this PR? like tons of changes are totally irrelevant to the content update

I'm not sure I agree about rewriting history on a public branch. This only happened a day ago and I don't think that changing it will affect anyone who isn't already involved in PRs on this repo anyway

gouttegd commented 1 year ago

@cthoyt Those “spurious changes” are not irrelevant: they are here to fix the real spurious changes that were introduced in #672. Obviously, when you introduced a spurious change, the fix to it also looks like a spurious change.

Note that if you go the #732 route and reset the main branch to before #672 was merged, then this PR becomes irrelevant, so I don’t know why your last item in the description of #732 is to ”fix #731”.

(I am personally opposed to any form of history re-writing on a public repo. That being said, I don’t know what is the OBO policy on the matter – if even there is one –, so if nobody else has a problem with that, I won’t object too much. All I want is to fix the master branch so that I can then update and merge #709.)

wdduncan commented 1 year ago

All I want is to fix the master branch so that I can then update and merge https://github.com/oborel/obo-relations/pull/709.

That is what is most important. Github purity is secondary to do this.

gouttegd commented 1 year ago

This only happened a day ago and I don't think that changing it will affect anyone who isn't already involved in PRs on this repo anyway

672 was merged 3 days ago. Hard-resetting the master branch to before it was merged will affect anyone who has updated their local copy since then. That’s reason enough for not doing so, in my opinion.

wdduncan commented 1 year ago

Thanks for your help @balhoff and @gouttegd !