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

Allow NamespaceManager "unbind" by passing None for a namespace #1932

Open
mwatts15 opened this issue May 15, 2022 · 4 comments
Open

Allow NamespaceManager "unbind" by passing None for a namespace #1932

mwatts15 opened this issue May 15, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@mwatts15
Copy link
Contributor

mwatts15 commented May 15, 2022

Currently, there's not a clean way to remove namespace bindings from a NamespaceManager. I'm proposing to allow NamespaceManager.bind(prefix, None, replace=True) to signal to Stores that they should clear the binding for the given prefix.

Currently, there isn't a sensible use for binding to None -- it creates a binding to the value URIRef("None") which probably isn't what's intended.

One alternative to this is defining a new method on NamespaceManager, say "unbind" or "remove", that does this. The advantage to that is that existing Store implementations wouldn't have to handle the None namespace case. The disadvantage is that it requires a new method to support the behavior although some (like mine) already support this.

A disadvantage of allowing unbind by passing None is that it's something Store impls would need some kind of marker attribute (e.g., bind_none_is_unbind) to indicate they support it to avoid sending them unexpected input. One advantage is that it's fairly simple to implement without changing the formal API (as opposed to what I'd call the "service provider interface" or "plugin interface" for Store implementations).

related: #543

@ghost
Copy link

ghost commented May 15, 2022

There are API-breaking changes in the pipeline (actually in current master, tbh) and unbind better preserves the “principle of least surprise”.

Glad to read that rdflib-sqlalchemy already supports unbind - I have uncommitted code that brings the other still-functional RDFLib stores¹ up to this spec.

I think that only leaves Thomas Tanon's oxrdflib

¹ basically: those in rdflib/plugins/stores and rdflib-leveldb. It's difficult to gauge the degree of actual interest in RDFLib back-end stores and there's more than enough to do in core RDFLib without unnecessarily adding little-used ancillary packages to the maintenance load - having asserted that caveat, I've already refreshed rdflib-leveldb,I have a refurbed rdflib-kyotocabinet to commit and a new rdflib-sqlitelsm to contribute. I'd also like to update the RDFLib rdflib-zodb repos with your changes.

FWIW: current state-of-play w.r.t. viable RDFLib persistence-backed stores - performance stats (time in seconds) on a graph of ~500k triples: ...

store loading opening length subjects q01 q11
KyotoCabinet 21.44823 0.00035 8.89224 0.00002 0.00306 6.66116
FileStorageZODB 118.33601 0.02040 5.16442 0.00002 0.00352 5.23339
LevelDB 70.65584 0.05667 8.87643 0.00002 0.00298 5.59449
OxSled 47.01792 12.02569 12.44463 0.00002 0.00029 2.23770
Oxigraph 50.36093 11.40930 9.82383 0.00002 0.00024 1.94353
SQLiteLSM 48.51349 0.00099 6.36751 0.00002 0.00337 6.42877
SQLAlchemy:MYSQL 40.50109 0.01340 14.57538 0.00006 0.03224 6.26159
SQLAlchemy:PGSQL 174.67257 0.09026 8.56737 0.00002 0.02358 5.87448
SQLAlchemy:SQLITE 22.53359 0.00251 6.93722 0.00002 0.01686 5.99237
BerkeleyDB 47.85095 0.00093 12.82477 0.00002 0.00427 6.78863
SQLiteDBStore 61.09035 0.00122 21.15709 0.00002 0.00315 7.07173

loading from an in-memory Graph, opening a previously-saved store, length len(list(graph.triples((None, None, None)))), subjects graph.subjects(predicate=RDF.type, object=FOAF.Person) and two sp2b SPARQL queries

@mwatts15
Copy link
Contributor Author

There are API-breaking changes in the pipeline

Referring to the new override option to bind? I noticed by chance.

Glad to read that rdflib-sqlalchemy already supports unbind

It doesn't. I was talking about a different store, but only w.r.t. accepting None for namespace to mean "unbind".

I'd also like to update the RDFLib rdflib-zodb repos with your changes.

You mean openworm/rdflib-zodb? You're welcome to. I'm unlikely to put much effort in merging back because I don't intend to go back to using the official rdflib-zodb.

@ghost
Copy link

ghost commented May 15, 2022

Referring to the new override option to bind? I noticed by chance.

Issue #1880 pertains

Glad to read that rdflib-sqlalchemy already supports unbind

It doesn't. I was talking about a different store, but only w.r.t. accepting None for namespace to mean "unbind".

Aha, my mistake, I grepped for def unbind in my local checkout of rdflig-sqlalchemy, found it but forgot it was me that added it 😄

I'd also like to update the RDFLib rdflib-zodb repos with your changes.

You mean openworm/rdflib-zodb? You're welcome to.

Thank you.

@mwatts15
Copy link
Contributor Author

mwatts15 commented May 22, 2022

Got somewhat off-track in this issue: Regarding "principle of least surprise", I don't think namespace_manager.bind("oldns", None) binding oldns to the string "None" is the least surprising, but that's what happens now. I guess I'm OK with it not unbinding, but if it's not an unbind, I think throwing a TypeError would be better. Really, I think anything that isn't a str being passed for the namespace URI should raise a TypeError in that case.

@aucampia aucampia added the enhancement New feature or request label Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants