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

Add N-Quads output to phyx.js #92

Merged
merged 6 commits into from
Mar 19, 2021
Merged

Add N-Quads output to phyx.js #92

merged 6 commits into from
Mar 19, 2021

Conversation

gaurav
Copy link
Member

@gaurav gaurav commented Mar 18, 2021

This PR replaces the PhyxWrapper.asOWLOntology() with two methods: PhyxWrapper.asJSONLD() for converting a Phyx file into an OWL/JSON-LD file, and PhyxWrapper.asNQuads() for converting a Phyx file into an N-Quads file, which we do by converting the OWL/JSON-LD file into N-Quads using the jsonld NPM package. While the OWL/JSON-LD files are very useful in development, debugging and testing, we anticipate that most users will use PhyxWrapper.asNQuads() to directly produce N-Quads. It also modifies the phyx2owl.js script so that it produces N-Quad files rather than OWL/JSON-LD files.

Note that we use "N-Quads" instead of "N-Triples" because it is possible to set up a subgraph within a Phyx file using JSON-LD's graph containers, in which case the jsonld NPM package we use should emit those graph IRIs in the N-Quads output. However, I don't know why anybody would do this, and in almost all cases the output we produce will be N-Triples rather than N-Quads. I think "N-Quads" is probably still the right term to use, since that format specifically says the graph IRI may be left out if not needed. But I just wanted to be clear about that.

Should be merged after PR #87.

@gaurav gaurav requested a review from hlapp March 18, 2021 18:20
Copy link
Member

@hlapp hlapp left a comment

Choose a reason for hiding this comment

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

My main suggestion is to consider renaming asNQuads() to asRDF(), or even toRDF(). It seems the primary purpose of this method is to produce RDF, and the fact that it accomplishes this by producing NQuads is secondary. Also, it seems safe to say that it will never produce NQuads that isn't RDF. Finally, it would keep it nicely consistent with the method name in the lsonld package.

Base automatically changed from delete-rdf-specifiers to master March 18, 2021 22:50
@gaurav
Copy link
Member Author

gaurav commented Mar 18, 2021

Sounds good! I've renamed PhyxWrapper.asNQuads() to PhyxWrapper.toRDF() in 84cd86f.

@gaurav gaurav requested a review from hlapp March 18, 2021 23:02
Copy link
Member

@hlapp hlapp left a comment

Choose a reason for hiding this comment

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

👍🏻

@gaurav gaurav merged commit 90edae6 into master Mar 19, 2021
@gaurav gaurav deleted the add-nq-output branch March 19, 2021 16:48
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.

2 participants