Improve typing of DataFactory

[RubenVerborgh]

No description provided.

[megoth]

Works mostly ok with maslib - there's one bug though, which is that it requires termToNQ in canonical-data-factory.ts to handle terms that are graphs. I think a possible solution is to handle GraphTermType same as NamedNodeTermType, but I'm not well versed enough in RDF to know for certain.

If you could fix the handling of terms with type 'Graph', I think we're all good with this PR.

I've invited @timbl to review this as well, as he is very interested in how we handle types in rdflib.

[RubenVerborgh]

Sorry, I don't fully understand:

Works mostly ok with maslib - there's one bug though, which is that it requires termToNQ in canonical-data-factory.ts to handle terms that are graphs.

• Is the bug that termToNQ requires handling graphs (as in: it should not)?
• Or is the bug that termToNQ does not handle graphs?

Do you have a code example of what breaks?

[megoth]

Sorry, I don't fully understand:

Works mostly ok with maslib - there's one bug though, which is that it requires termToNQ in canonical-data-factory.ts to handle terms that are graphs.

* Is the bug that `termToNQ` requires handling graphs (as in: it should not)?

* Or is the bug that `termToNQ` does not handle graphs?

Do you have a code example of what breaks?

Sorry for my imprecisessness - if I build the data browser with the current changes, I get the following error:

Error: Can't serialize nonstandard term type (was 'Graph')
  https://megoth.localhost:8443/mashlib.js:7308:15
    termToNQ  https://megoth.localhost:8443/mashlib.js:7230:23
    id  https://megoth.localhost:8443/mashlib.js:7432:42
    id  https://megoth.localhost:8443/mashlib.js:10100:30
    id  https://megoth.localhost:8443/mashlib.js:19608:38
    canon  https://megoth.localhost:8443/mashlib.js:20227:27
    statementsMatching  https://megoth.localhost:8443/mashlib.js:10197:22
    each  https://megoth.localhost:8443/mashlib.js:67316:24
    outlineObjectTD  https://megoth.localhost:8443/mashlib.js:4114:35
    refresh/<  https://megoth.localhost:8443/mashlib.js:90436:32
    syncTableToArray  https://megoth.localhost:8443/mashlib.js:4107:18
    refresh  https://megoth.localhost:8443/mashlib.js:4121:7
    render  https://megoth.localhost:8443/mashlib.js:67943:56
    renderPane  https://megoth.localhost:8443/mashlib.js:67975:47
    listen/<  undefined

I've pushed a possible solution to feature/extended-typing-toNQ-fix, but not sure if this is correct.

[RubenVerborgh]

Ok seems like we're still doing

export const CollectionTermType = "Collection" as const
export const EmptyTermType = "Empty" as const
export const GraphTermType = "Graph" as const

stuff, which we really should not.

However, my main mission was to unblock you on #397, which I think I've achieved.

I think the route for fixing the problem is getting rid of GraphTermType, as there really is no such thing; they are either named, blank, or default. In the meantime, workaround extended-typing-toNQ-fix will do.

[megoth]

Ok seems like we're still doing

export const CollectionTermType = "Collection" as const
export const EmptyTermType = "Empty" as const
export const GraphTermType = "Graph" as const

stuff, which we really should not.

Should we create a ticket to track this?

However, my main mission was to unblock you on #397, which I think I've achieved.

Yes, thank you again 😸

I think the route for fixing the problem is getting rid of GraphTermType, as there really is no such thing; they are either named, blank, or default. In the meantime, workaround extended-typing-toNQ-fix will do.

Yeah, I think we should create a ticket to track the issue, and then remove the hack introduced in feature/extended-typing-toNQ-fix.

I've created #407 that can be merged when everything's in order.

[pmcb55]

Just my 2cents, but perhaps this strange looking 'Graph' type is one of the N3 types that Tim mentioned were extensions to the standard RDF types...!?

[RubenVerborgh]

It's a misnomer in any case then. Notation3 has formulas.

[timbl]

It's a misnomer in any case then. Notation3 has formulas.

N3 has things it calls formulas and other people call graphs. So not really a misnomer. (Like Symbol/NamedNode, Formula/Graph, Statement/Arc, etc these are differences between talking about it as a logic language and as a graph. It is useful to be able to look at it either way, as for core RDF a lot of people thing of the graph, but for N3 you have a language which is clean superset of the RDF but where is more than a graph)

I suspect the solution is to check if the given thing is of termtype Graph, and if so raise a run time value error, saying ' This is N3 data, it is too complex to be serialized as an NQuad item" or words to that effect. I always thought we would need run-time errors when we build the system which mix N3 and and base RDF. It is fine to have people use rdflib.hs most of the time for plain RDF, but it would be be bad for the types they are using not t allow them to be able to express more complex things wher ethey ned to. JUst f they try to serialize a query or a rule etc in say turtle ot RDF/XML the system will raise a run time error not a compile time type mismatch.

[RubenVerborgh]

I suspect the solution is to check if the given thing is of termtype Graph, and if so raise a run time value error, saying ' This is N3 data, it is too complex to be serialized as an NQuad item" or words to that effect.

I think a specific termtype for Graph neither necessary nor desired.

Graphs are identified by URIs (or blank nodes), so I'm missing arguments as to why they would be a separate thing.

As a note, N3.js parses Notation3 and remains within the RDF/JS model.

[elf-pavlik]

I've noticed #404, maybe it would be possible to have rdflib.js stick to RDF and have distinct n3.js depend on it and extend it with N3 specific functionality. I can imagine developers intending to use rdflib.js to handle RDF getting confused if they see some N3 errors thrown at them.

[megoth]

I think this is ready to be merged into master. Does anyone object to this? (Pinging @timbl)

[megoth]

Seems that we need to fix one more type error that was hidden with @ts-ignore, namely an incorrect extension of Literal: https://github.com/linkeddata/rdflib.js/blob/master/src/literal.ts#L16

Will try to fix this tonight to get this merged and a new version of rdflib released.

[megoth]

Will try to fix this tonight to get this merged and a new version of rdflib released.

An ugly fix possible, pushed as #414. If this is ok, I think I should be able to merge this to master and release a new version of rdflib.