[GUI Plugin] Action Chains

[capink]

The single field edit actually calls set_field() once per field for all book_ids in the scope (single call per field). But if you use multiple single field actions for different fields, it will call set_field() for each field. AFAIK you cannot call set_field() for multiple fields.

Edit: Also if for some reason a user is changing the same field more than once in the same chain (I don't know why anyone would do this), he can use the set_book_vars(), and commit only the final change at the very end using single field edit.

[chaley]

Originally Posted by capink

AFAIK you cannot call set_field() for multiple fields.

You can set multiple fields for a single book using set_metadata().

I don't know at what point, if any, iterating over books calling set_metadata() would be faster than iterating over fields calling set_field().

[capink]

I would guess iterating over fields using set_field() is faster than iterating over books with set_metadata() in most situations. noteworthy is that Calibre's mass metadata edit calls set_field() regardless of the number of books it operates on. Will do some tests and see how it goes.

[chaley]

Originally Posted by capink

noteworthy is that Calibre's mass metadata edit calls set_field() regardless of the number of books it operates on.

But with auto-commit turned off so the transactions are grouped into one commit.

Will do some tests and see how it goes.

Thanks.

[queensacrifice]

I think the template scopes dialog might not be registering as exited properly for me because after editing it, the default template dialog values rather than book values are retrieved for from_selection() until I open and exit a different template dialog, such as formulas.

[ownedbycats]

If it'll help for testing purposes, here's my most complicated cleanup chain and an empty test DB with all the needed columns. (If you need books, I can add a few.) Note that you might need to remove the first "clean comments" as it's plugin.

[capink]

I made a quick test with set_metadata and it was slower than single field edit even for one book. It compounded the problem and calls Last Modified plugin multiple times per field per book.

from calibre_plugins.action_chains.actions.base import ChainAction


class Test(ChainAction):

    name = 'Test set_metadata'
    support_scopes = True

    def run(self, gui, settings, chain):
        book_ids = chain.scope().get_book_ids()
        
        db = gui.current_db
        cache = db.new_api
        
        if len(book_ids) == 0:
            return

        for book_id in book_ids:
            #mi = cache.get_proxy_metadata(book_id)
            mi = cache.get_metadata(book_id)
            mi.set('#test1', 'text')
            cache.set_metadata(book_id, mi)

Edit: It seems set_metadata() is calling set_field() for every field, even if the field value was not changed.

[capink]

Originally Posted by queensacrifice

I think the template scopes dialog might not be registering as exited properly for me because after editing it, the default template dialog values rather than book values are retrieved for from_selection() until I open and exit a different template dialog, such as formulas.

I don't understand the issue. Can you please elaborate more clearly by giving steps to replicate the issue you are trying to describe?

[capink]

Originally Posted by capink

I made a quick test with set_metadata and it was slower than single field edit even for one book. It compounded the problem and calls Last Modified plugin multiple times per field per book.


Edit: It seems set_metadata() is calling set_field() for every field, even if the field value was not changed.

set_metadata() completely wrecks havoc with Last Modified plugin because it overwrites all the fields, even if the value has not changed, and emits db event, falsely notifying Last Modified plugin about modification in those fields.

Problem is, set_metadata() is used by a number of plugins (e.g. Import List). The solution for this mess is to retire the Last Modified plugin, because in the current state, it cannot do its advertised job.

If anyone knows what is required to retire a plugin, please tell me. Is it enough to simply remove from the Index of plugins thread? Or is there something else I have to do?

[chaley]

@capink: the right fix is to change calibre base so that it doesn't set values equal to themselves. This already happens for some fields like tags.

I have been looking at the field writers and have been able to make this work with all fields except custom series indexes. Problem: I don't know what I might be breaking, so Kovid needs to be involved. I will continue to work on this but I don't know what my constraints are.

I suggest you open a bug report suggesting that

set_metadata(id_, get_metadata(_id))

shouldn't signal any dirty books and see what he says.

[capink]

@chaley: Thanks for your response and help. I will have a look at it, and open the bug report, probably tomorrow, even though I've already bugged Kovid enough about this.

[chaley]

Originally Posted by capink

@chaley: Thanks for your response and help. I will have a look at it, and open the bug report, probably tomorrow, even though I've already bugged Kovid enough about this.

At least this report is as much about performance as avoiding the changed events. In some cases updating a field without changing the value actually updates the database, which I imagine should be avoided.

Tomorrow I hope to be able to push changes for review. I don't expect the changes to be complete or even correct, but they should provide something to work with.

With the changes I have made you get changed events for the set_field() operations, but with an empty book set. I don't know if it is worth trying to filter those out at the source. I have a test function that runs over a given library calling

set_metadata(id_, get_metadata(id_))

on each book. The test works if all changed events have an empty book set.

[capink]

Originally Posted by chaley

I suggest you open a bug report suggesting that

set_metadata(id_, get_metadata(_id))

shouldn't signal any dirty books and see what he says.

I tried this and it does not make a difference, the set_metadata() still calls set_field() for every field, which in turn will dispatch a db event. I think I may have misunderstood you here.

Originally Posted by chaley

With the changes I have made you get changed events for the set_field() operations, but with an empty book set. I don't know if it is worth trying to filter those out at the

I don't think it is worth filtering out. At least in my case, they will be discarded.

[chaley]

Originally Posted by capink

I tried this and it does not make a difference, the set_metadata() still calls set_field() for every field, which in turn will dispatch a db event. I think I may have misunderstood you here.

This tests whether setting a book's metadata to itself generates any events with a non-empty set of books. In particular it tests whether set_field() thinks the book was dirtied when in theory it shouldn't have been.

Originally Posted by capink

I don't think it is worth filtering out. At least in my case, they will be discarded.

TUrns out that the it is easy to filter the events in set_field() with no performance penalty.

EDIT: I think that in a couple of hours I will have code you can test.

[chaley]

Here are changes to make set_field() not signal dirty books if nothing actually changed. If they work for you then I will make a pull request to give them to Kovid. The changes in formatter_functions aren't part of the main changes, but instead make testing for being in the gui more robust.

My test function does set_metadata(get_metadata()) on evey book in the

target library. Run it with calibre-debug -e test.py path-to-library

Note that it really does update metadata so you should keep a backup if you want repeatable tests. For example, in one of my libraries setting the comments field did some cleanup so a change was signaled. On the second run no cleanup was needed and no change was signaled.

I also checked the changes using the calibre test suite.