Changing the graph.add() method parameters

[ashleysommer]

It seems to be a fairly largely held annoyance that graph.add() takes a tuple object.

I.e., it feels clunky do to:
g.add((subject, predicate, object)) with the double parentheses, when we should be able to do:
g.add(subject, predicate, object)

RDFLib 6.0.0 coming soon has some breaking changes, so now would be the time to change this if we want to change it.
The problem is that even though we can have some rational breaking changes, making this change outright with no backwards compatibility would break literally every rdflib application, script, and workflow ever written.

So I've been doing some benchmarks of how to add a layer of backward compatibility without affecting performance too much.
These are my findings. (in the following comments)

[ashleysommer]

Candidate Method Implementations

This is the original add() method. For baseline reference. In this test, it is named add_orig():

    def add_orig(self, triple):
        s, p, o = triple
        assert isinstance(s, Node), "Subject %s must be an rdflib term" % (s,)
        assert isinstance(p, Node), "Predicate %s must be an rdflib term" % (p,)
        assert isinstance(o, Node), "Object %s must be an rdflib term" % (o,)
        self.__store.add(triple, self, quoted=False)

This next one is add_ref2(). It straight-up takes a non-tuple form as argument. It is not backward compatible. Using this as a best-case metric.

    def add_ref2(self, s, p, o):
        assert isinstance(s, Node), "Subject %s must be an rdflib term" % (s,)
        assert isinstance(p, Node), "Predicate %s must be an rdflib term" % (p,)
        assert isinstance(o, Node), "Object %s must be an rdflib term" % (o,)
        self.__store.add((s, p, o), self, quoted=False)

This next one is add_newA(). It can take s, p, o as args, or (s,p,o) as a tuple. If s is a tuple, it unpacks it to s,p,o. If s is not a tuple, it asserts that p and o are not None.

    def add_newA(self, s, p=None, o=None):
        if isinstance(s, tuple):
            s, p, o = s
        else:
            assert p is not None
            assert o is not None
        assert isinstance(s, Node), "Subject %s must be an rdflib term" % (s,)
        assert isinstance(p, Node), "Predicate %s must be an rdflib term" % (p,)
        assert isinstance(o, Node), "Object %s must be an rdflib term" % (o,)
        self.__store.add((s, p, o), self, quoted=False)

Finally, add_newB(). It is very similar to the above, can take s, p, o as args, or (s,p,o) as a tuple. If s is a tuple, calls the add_orig method. This one does not check for that p and o are not None.

    def add_newB(self, s, p=None, o=None):
        if isinstance(s, tuple):
            return self.add_orig(s)
        assert isinstance(s, Node), "Subject %s must be an rdflib term" % (s,)
        assert isinstance(p, Node), "Predicate %s must be an rdflib term" % (p,)
        assert isinstance(o, Node), "Object %s must be an rdflib term" % (o,)
        self.__store.add((s, p, o), self, quoted=False)

[ashleysommer]

Testing methodology:

Similar to how I benchmarked the new in-memory store. I generate 10,000 random unique triples.
I then use the python timeit utility to try each of the add() implementations above, with different argument types.
Each test is run 15 times, then the total elapsed time for each run divided by 15.0 before displaying the result.

The runs are:

• Ref1: add_orig() passing triple as a tuple, this represents current performance level
• Ref2: add_ref2() passing triple as s, p, and o args, this represents a best-case performance level (no backward compat)
• TestAa: add_newA() passing triple as s, p, and o args
• TestAb: add_newA() passing triple as a tuple to test backward compat.
• TestBa: add_newB() passing triple as s, p, and o args
• TestBb: add_newB() passing triple as a tuple to test backward compat.

[ashleysommer]

Results:

On Python 3.6:

Ref1:     0.1145 seconds
Ref2:     0.1138 seconds
Test Aa:  0.1151 seconds
Test Ab:  0.1145 seconds
Test Ba:  0.1146 seconds
Test Bb:  0.1168 seconds

On Python 3.7:

Ref1:     0.1102 seconds
Ref2:     0.1082 seconds
Test Aa:  0.1096 seconds
Test Ab:  0.1084 seconds
Test Ba:  0.1104 seconds
Test Bb:  0.1090 seconds

On Python 3.8:

Ref1:     0.0876 seconds
Ref2:     0.0897 seconds
Test Aa:  0.0898 seconds
Test Ab:  0.0896 seconds
Test Ba:  0.0887 seconds
Test Bb:  0.0892 seconds

[ashleysommer]

Interpretation:

On Python 3.6:
The difference in runtime between each of the different tests, to add 10,000 triples using each implementation is within around 1% of each other.
As expected the quickest is the non-backward-compatible Ref2 implementation, and the worst-performing is the TestBb, where we pass a tuple to add_newB() which in-turn redirects the call to add_orig().
TestAa, TestAb, and TestBa are all in between, and are approximately equal in run-time, with difference in speed in comparison to the best-case ref, of less than 1%.

On Python 3.7:
Results are similar to those on Python 3.6, but timings are tighter, and differences are smaller.

On Python 3.8:
Each of the implementations are executing so fast, these numbers are meaningless.

[ashleysommer]

Recommendation:

After seeing these results, I see there is no reason we can't change the implementation of add() to match the add_newA() or add_newB() implementation above.
The same change will likely be applied to the g.remove() method too.

Remaining questions:

• Will we remove the ability to take a tuple object in a post-6.0.0 version of RDFLib?
• If so, do we need to throw a deprecation warning every time a user uses g.add() with a tuple? That would further slow down the method, and create a tonne of warnings if someone is adding thousands or millions of triples.
• If we don't throw the deprecation warning, how will users now they should be moving to using s, p, o args?
• Is there an alternative to making p and o optional params? A method signature like add(s, p=None, o=None) is clunks and non-intuitive, but it seems like that's the only way to allow passing either 1 or 3 args.
• Does this need to be done on graph.addN() too?
• Will this same change be able to be applied to ConjunctiveGraph and Dataset too? It probably can be, but I haven't tested.

[tgbugs]

Chiming in here briefly. When I started using rdflib now over 4 years ago I also balked at the fact that I had to work with whole triples, so I wrote a wrapper that would let me pass s, p, o in directly. In retrospect this was a mistake. There are indeed a number of cases where one might like to pass s, p, o individually, but in another number of cases you definitely do not want to do this because the code you are working with operates on whole triples. The end result of this change will be needless work pushed on to consumers of the rdflib api forced to add * to unpack tuples all over their code base, and potential breaking of packages that no longer have a maintainer. The balance between use cases where a triple is passed vs s, p, o is probably fairly even, so changing this is just a cost, and not a benefit to users as a whole, and that is before considering the changes that this would force onto many existing code bases. Many of the use cases that I encountered for passing s, p, o were the early notebook exploratory style use cases. If such an api is desired or needed, then I would suggest that rdflib provide it under a new name.

[ghost]

Chiming in here briefly. When I started using rdflib now over 4 years ago I also balked at the fact that I had to work with whole triples, so I wrote a wrapper that would let me pass s, p, o in directly. In retrospect this was a mistake. ... Many of the use cases that I encountered for passing s, p, o were the early notebook exploratory style use cases.

I think this is an extremely valuable and useful insight. As advertised, “RDFLib is a Python library for working with RDF”. Its auxiliary role of offering a user-friendly interface to exploring/working with RDF directly is indeed valuable but it isn‘t RDFLib’s primary focus. Such a profound modification of a core library call seems a high price to pay for a what seems to be a minor degree of UI felicity.