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

Fix prov ns #1318

Merged
merged 2 commits into from Jun 26, 2021
Merged

Fix prov ns #1318

merged 2 commits into from Jun 26, 2021

Conversation

nicholascar
Copy link
Member

Fixes #1308

Proposed Changes

  • added missing terms
  • sorted all terms

@aucampia please review since you motivated this fix. Just want you to make sure I got it right.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.705% when pulling 0622dd3 on fix_prov_ns into a9aaef1 on master.

5 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.705% when pulling 0622dd3 on fix_prov_ns into a9aaef1 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.705% when pulling 0622dd3 on fix_prov_ns into a9aaef1 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.705% when pulling 0622dd3 on fix_prov_ns into a9aaef1 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.705% when pulling 0622dd3 on fix_prov_ns into a9aaef1 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.705% when pulling 0622dd3 on fix_prov_ns into a9aaef1 on master.

@nicholascar
Copy link
Member Author

This PR is failing tests with the "Cannot assign requested address" error so it the reviews are OK, I'll merge it (i.e. don't take the tests seriously).

Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I ran a check with: Namespace.test_prov and it passes, also check sorting.

With this change rdflib.namespace.PROV will contain all terms matching [] rdfs:isDefinedBy <http://www.w3.org/ns/prov-o#> from http://www.w3.org/ns/prov.ttl.

@aucampia
Copy link
Member

For reference the exact test I ran against it:

    def test_prov(self) -> None:
        graph = Graph()
        graph.parse(source=(SCRIPT_PATH.parent / "data" / "prov.ttl"), format="turtle")

        qres = graph.query(
            r"""
            PREFIX owl: <http://www.w3.org/2002/07/owl#>
            SELECT DISTINCT ?term ?ontology WHERE {
                ?term ?p [].
                # MINUS { ?term rdf:type owl:AnnotationProperty }
                FILTER isIRI(?term).
                FILTER strStarts(str(?term), "http://www.w3.org/ns/prov#").
                FILTER (str(?term) != "http://www.w3.org/ns/prov#")
                OPTIONAL { ?term rdfs:isDefinedBy ?ontology }
            }
            """
        )
        owl_terms = {}
        for row in qres:
            owl_terms[row["term"]] = row["ontology"]
        owl_term_set = set(owl_terms.items())

        provo_uri = URIRef("http://www.w3.org/ns/prov-o#")
        # provo_term_set is terms matching :
        #   [] rdfs:isDefinedBy  <http://www.w3.org/ns/prov-o#>
        provo_term_set = set([term for term in owl_term_set if term[1] == provo_uri])

        rdflib_term_set = set(
            [
                (PROV[termword], owl_terms.get(PROV[termword], None))
                for termword in dir(PROV)
            ]
        )
        self.assertLessEqual(provo_term_set, rdflib_term_set)
        self.assertGreaterEqual(owl_term_set, rdflib_term_set)

I won't mind making a PR for it, but it is rather slow and does not necessarily make sense to run every time, could maybe add them conditional on an environment variable like TEST_PEDANTIC or TEST_SLOW, so that if either of those are set the test runs.

@nicholascar
Copy link
Member Author

Merging as the only error is the old annoying one of not being able to assign an address (i.e. not a real error).

@nicholascar nicholascar merged commit a83aa06 into master Jun 26, 2021
@nicholascar nicholascar deleted the fix_prov_ns branch June 26, 2021 03:31
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.

Potentially missing terms in PROV namespace
3 participants