pchampin / sophia_rs

Sophia: a Rust toolkit for RDF and Linked Data
Other
210 stars 23 forks source link

Remove 'Namespace' #62

Closed MattesWhite closed 4 years ago

MattesWhite commented 4 years ago

Namespace had only one functionality and was practically another Iri-type.

This PR removes the Namespace and adds Iri::with_suffix() which has the same semantics.

The reason for this change is because conversion between Namespace and Iri was complicated and not necessary. In addition, improvements to Iri would have been required to be implemented for Namespace as well which no one has done. However, as Namespace only had a single function it was easier to move this function to Iri.

pchampin commented 4 years ago

Thanks for this proposal. I don't think I agree with your premise "Namespace was practically another Iri-type":

Furthermore, schema.get("Person") looks much nicer, I think, than Term::from(schema.with_suffix("Person")).

I could see the value of providing From conversion from Iri to Namespace, and possibly a get_iri method in Namespace, in addition to get... But as for replacing Namespace with Iri entirely... not convinced yet, sorry.

MattesWhite commented 4 years ago

Okay naming Namespace another IRI-type were maybe not the best words. In fact, some transitions between Namespace and Iri might be more reasonable.

let var = Term::from(schema.with_suffix("Person")?);
// maybe better looking
let var: Term<_> = schema.with_suffix("Person")?.into();

The thing is I would really like to return Iri instead of Term. The reason is re-usability. Of course we could do a .try_into() if we need an IRI but this would introduce another error-handling.


To give some context:

I'm currently updating metis to the current state of sophia. This will also be the PoC of how a Term-trait could benefit sophia (see #55). I planned to use Namespace for the prefixes. However, after parsing a prefix's IRI it must be resolved against the current base IRI. Meaning (I) parse the IRI, (II) resolve the Iri and finally (III) convert Iri to Namespace. Hence, the PR. Maybe a impl Resolve<Namespace> for IriParsed could also help :thinking:.

Ideally, I could reuse the prefix-parser from Turtle for N3. Which means that I would like to return an Iri as Turtle and N3 use different terms which both implement From<Iri>. In fact, my N3Term looks something like:

enum N3Term<TD> {
    Iri(Iri<TD>),
    Literal(Literal<TD>),
    Existential(BlankNode<TD>),
    Universal(Variable<TD>),
    Formula(Formula<TD>), // some kind of `Graph<N3Term>`
    List(Vec<N3Term<TD>>),
}

As you can see I would like to reuse a Graph implementation of sophia which I can't do because it's fixed to sophia_term::Term.

pchampin commented 4 years ago

Maybe a impl Resolve<Namespace> for IriParsed could also help :thinking:

Yep, that would look much better to me. This and a get_iri method on Namespace, so that you can get an Iri instead of a Term...

MattesWhite commented 4 years ago

Once again a good discussion :+1:

I committed a new PR (#63) that applies the changes discussed. Therefore, I close this PR.