Allow to return the number of triples added from Graph.addN

[mwatts15]

Some triple stores can tell you whether the triple you requested to add actually resulted in a new entry in the store. Knowing this information is helpful for debugging, informing users, and optimization (e.g., sizing buffers). It would be helpful to return such information from the addN method especially since, as a bulk operation, the only way to determine this information otherwise would be iterating over the entries twice, once to get the number that would be added and then to actually add.

[nilay77]

Hi, I would like to solve this issue. Please guide me on what changes need to be made. Thanks!

[mwatts15]

Probably, just return the values from stores for add() and addN() in Graph and sub-classes. It would also be good to have at least one of the built-in stores (e.g., IOMemory) return the number of triples added. To rdflib maintainers (@nicholascar, @joernhees, etc): there may be scope for a generic 'AddResult' or 'AddNResult' rather than just the number, in case there's more info a store can provide: any thoughts on that?

…

On Tue, May 26, 2020, 07:55 nilay77 ***@***.***> wrote: Hi, I would like to solve this issue. Please guide me on what changes need to be made. Thanks! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#833 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALLFSFWTF6RZXNCWFG6SNTRTO32LANCNFSM4FFONGNA> .

[nicholascar]

there may be scope for a generic 'AddResult' or 'AddNResult' rather than just the number

What other info might be interesting? Counts of unique subjects, counts of URIs v. Literals etc? I wonder what precedence in the toolkit is for complex returns like this? It could be a small graph...

@mwatts15 also, regarding implementation, in the description you said: "the only way to determine this information otherwise would be iterating over the entries twice" but how would an implementation here be any different? Wouldn't the implementation have to do something like this:

def add(...)
    count_before = len(g)
    # add the triples
    count_after = len(g)

    return count_after - count_before

So this is still a double count of g.

Unless if you mean the back-end store might have some native way of doing the len(g) work and this function passes back to them, so the above implementation would be in-memory only but able to be overridden by other stores.

[mwatts15]

On Wed, May 27, 2020, 00:39 Nicholas Car ***@***.***> wrote: Unless if you mean the back-end store might have some native way of doing the len(g) work and this function passes back to them, so the above implementation would be in-memory only but able to be overridden by other stores I meant this. rdflib-zodb, for instance, uses a set data structure that

returns this info from its 'insert' method. I definitely want this to be an optional interface that depends entirely on the store implementation rather than providing a default in Graph just because I know some stores don't take care to or can't get this statistic efficiently. Also, I had been ambivalent about this but a marker like 'supports_modification_stats' on the store to opt-in to this would be good in case there's some stores out there returning values from addN already for their own purposes.

…

[nilay77]

@mwatts15 Is there any problem with the solution @nicholascar proposed? As far as I understood, the time complexity of len(g) depends on how the __len__ method has been implemented. In the case of Graph, this method simply calls the __len__ of the store it uses: here. Now in case of the default IOMemory store, the __len__ method returns the length of a dictionary: here, and as I understand this operation is O(1).
The problem lies with the __len__ method of the other Memory store. It uses a for loop to iterate over all the triples and then return the count: here. So should I work on optimizing the latter method and then add the solution proposed by @nicholascar?

[mwatts15]

On Thu, May 28, 2020, 03:26 nilay77 ***@***.***> wrote: @mwatts15 <https://github.com/mwatts15> Is there any problem with the solution @nicholascar <https://github.com/nicholascar> proposed? As far as I understood, the time complexity of len(g) depends on how the __len__ method has been implemented. In the case of Graph, this method simply calls the __len__ of the store it uses: here <https://github.com/RDFLib/rdflib/blob/037ea51e5f4863a7f98ff59972fcd34d39a7ed97/rdflib/graph.py#L496>. Now in case of the default IOMemory store, the __len__ method returns the length of a dictionary: here <https://github.com/RDFLib/rdflib/blob/037ea51e5f4863a7f98ff59972fcd34d39a7ed97/rdflib/plugins/memory.py#L379>, and as I understand <https://stackoverflow.com/questions/1115313/cost-of-len-function> this operation is O(1). The problem lies with the __len__ method of the other Memory store. It uses a for loop to iterate over all the triples and then return the count: here <https://github.com/RDFLib/rdflib/blob/037ea51e5f4863a7f98ff59972fcd34d39a7ed97/rdflib/plugins/memory.py#L148>. So should I work on optimizing the latter method and then add the solution proposed by @nicholascar <https://github.com/nicholascar>?

The issue I pointed out is just to avoid arbitrary stores suddenly paying a tax on their use with a Graph to support a feature that they didn't ask for and may not care about. If you want to make Memory's __len__ cheaper, I guess you can, but I think that's orthogonal to this issue. My suggestion to implement this interface in one of the bundled stores is to give guidance to RDFLib's users *in addition to documentation* on how to implement it themselves.

…

[mwatts15]

On Thu, May 28, 2020, 03:26 nilay77 ***@***.***> wrote: @mwatts15 <https://github.com/mwatts15> Is there any problem with the solution @nicholascar <https://github.com/nicholascar> proposed? As far as I understood, the time complexity of len(g) depends on how the __len__ method has been implemented. In the case of Graph, this method simply calls the __len__ of the store it uses: here <https://github.com/RDFLib/rdflib/blob/037ea51e5f4863a7f98ff59972fcd34d39a7ed97/rdflib/graph.py#L496>. Now in case of the default IOMemory store, the __len__ method returns the length of a dictionary: here <https://github.com/RDFLib/rdflib/blob/037ea51e5f4863a7f98ff59972fcd34d39a7ed97/rdflib/plugins/memory.py#L379>, and as I understand <https://stackoverflow.com/questions/1115313/cost-of-len-function> this operation is O(1).

If you are proposing to do this in *Graph*, I wouldn't be in favor of that. I would rather the Store knows what's being asked for so it can get the count in whatever way it thinks is cheapest. OTOH, if you want to use __len__ in IOMemory to compute the number of added triples, I've got no problem with that. Your analysis of the time complexity looks right to me. IOMemory's __len__ returns the size of a set rather than a dict.

…