Patch for small enhancement of functionality.

[kacir]

Dear developers.

For a long time I had a wish that there was a functionality like "Similar books" -> "Books by the same author", but it would search just author surname and it wouldn't use the default search string author:"=FirstName Surname", but only author:"Surname".

So today, I got pisse^H^H^H^H^Hmotivated enough and I made a change [jumping excitedly up and down].
I have downloaded the source, figured the way to do the 'print "Hello World"' change from Tutorial and run the result. Then I located the desired piece of code and I have made the change. It seems to work. I am surprised how quickly it went. I was expecting that it would take me *much* longer to figure it all out.

It is my first ever change to Calibre and I am pretty sure that my effort is very similar to a blind, lame elephant blundering through the proverbial china shop. But, now I have my very own customised version of Calibre that does exactly what I wanted it to do.

Sorry for not submitting a "proper" patch, but that is something I have never done before, so here is my change in a form of one commented changed source file.

File: /home/kacir/work/calibre/src/calibre/gui2/actions/similar_books.py

#!/usr/bin/env python
# vim:fileencoding=UTF-8:ts=4:sw=4:sta:et:sts=4:ai

__license__   = 'GPL v3'
__copyright__ = '2010, Kovid Goyal <kovid@kovidgoyal.net>'
__docformat__ = 'restructuredtext en'

from functools import partial

from PyQt4.Qt import QToolButton

from calibre.gui2.actions import InterfaceAction

#### Begin added by Kacir ####
import re
# I am not sure if this is really necessary, but it works this way. Most probably the re module is imported somewhere else, but I am not skilled in Python programming
#### END added by Kacir ####

class SimilarBooksAction(InterfaceAction):

    name = 'Similar Books'
    action_spec = (_('Similar books...'), None, None, None)
    popup_type = QToolButton.InstantPopup
    action_type = 'current'
    action_add_menu = True

    def genesis(self):
        m = self.qaction.menu()
        for text, icon, target, shortcut in [
        (_('Books by same author'), 'user_profile.png', 'authors', _('Alt+A')),
        #### Begin added by Kacir ####
        (_('Books by same author surname'), 'user_profile.png', 'surname', _('Alt+W')),
        #### END added by Kacir ####
        (_('Books in this series'), 'books_in_series.png', 'series',
            _('Alt+Shift+S')),
        (_('Books by this publisher'), 'publisher.png', 'publisher', _('Alt+P')),
        (_('Books with the same tags'), 'tags.png', 'tag', _('Alt+T')),]:
            ac = self.create_action(spec=(text, icon, None, shortcut),
                    attr=target)
            m.addAction(ac)
            ac.triggered.connect(partial(self.show_similar_books, target))
        self.qaction.setMenu(m)

    def show_similar_books(self, type, *args):
        search, join = [], ' '
        idx = self.gui.library_view.currentIndex()
        if not idx.isValid():
            return
        row = idx.row()
        if type == 'series':
            series = idx.model().db.series(row)
            if series:
                search = ['series:"'+series+'"']
        elif type == 'publisher':
            publisher = idx.model().db.publisher(row)
            if publisher:
                search = ['publisher:"'+publisher+'"']
        elif type == 'tag':
            tags = idx.model().db.tags(row)
            if tags:
                search = ['tag:"='+t+'"' for t in tags.split(',')]
        elif type in ('author', 'authors'):
            authors = idx.model().db.authors(row)
            if authors:
                search = ['author:"='+a.strip().replace('|', ',')+'"' \
                                for a in authors.split(',')]
                join = ' or '
        #### Begin added by Kacir ####
        elif type == 'surname':
            authors = idx.model().db.authors(row)
            if authors:
                search = ['author:"'+re.sub(r'(.*) ([^ ]*)',r'\2',a.strip().replace('|', ','))+'"' \
                                for a in authors.split(',')]
                join = ' or '
        #### END added by Kacir ####
        if search:
            self.gui.search.set_search_string(join.join(search),
                    store_in_history=True)

Is there a chance of this little patch (after being cleared tidied and vetted by experts, of course) making into official tree? Pretty please.

[kiwidude]

The first problem I see are the technical issues and many debates that flare up from time to time over the definition of "what is a last name". Did you consider the fact that some people store author as "Lastname, Firstname"? Or issues with names from other languages?

However the other issue I would suggest is that I don't see the use case for it?

The only situation that this functionality is presumably being used is when you are trying to find variations of an author's name to do a cleanup of your database. However there is a plugin that will do a better job of this already - Find Duplicates. Use that with an "Ignore Title, Similar Author" search to find your author variations, get them fixed, and from that point on the built-in Alt+A search will do you just fine. The other bonus of Find Duplicates is that it should find variations of combinations of the author names where the ordering may differ etc.

Just my 2p, but I have no say on what goes into the codebase so you can take that for what it is worth

[kacir]

Originally Posted by kiwidude

The only situation that this functionality is presumably being used is when you are trying to find variations of an author's name to do a cleanup of your database.

This is exactly how I am using it

Originally Posted by kiwidude

However there is a plugin that will do a better job of this already - Find Duplicates. Use that with an "Ignore Title, Similar Author" search to find your author variations, get them fixed, and from that point on the built-in Alt+A search will do you just fine.

I am aware of your Find Duplicates plugin, and I have used it extensively when cleaning my database, but this is a little bit better for my workflow.

I will see what answer I get regarding inclusion into main tree and then I might try to turn this into a plugin, so I do not have to modify Calibre every time there is a new version.

[kovidgoyal]

I'd rather not add this to trunk. As kiwidude points out, the duplicates plugin exists already and there are various issues with parsing author names that I'd rather not get into.

Don't worry your patch will remain functional through calibre upgrades. Just remember to do a

bzr merge

in your calibre source checkout when updating calibre.

[kacir]

Originally Posted by kiwidude

Did you consider the fact that some people store author as "Lastname, Firstname"?

There is a setting somewhere telling Calibre how you store Author name. So I could create two regular expressions for extracting surname depending on this setting. If there is enough interest.

[kacir]

Originally Posted by kovidgoyal

Don't worry your patch will remain functional through calibre upgrades. Just remember to do a

bzr merge

Now I am happy!

But first I will have to get the source "the proper way" by doing 'bzr branch lp:calibre' ;-) and apply my patch again manually. Then I can happily update my Calibre using bzr merge every Friday evening.
This might even be better regarding the file size as compared to binary update.

By the way, I was surprised how well the source is organised. I had much less problems setting the development stuff and doing my ugly patch than I expected.

Thank you again.

[kovidgoyal]

Having well organised source code is what makes it possible to develop/maintain calibre so rapidly with so few resources

[kiwidude]

Originally Posted by kacir

There is a setting somewhere telling Calibre how you store Author name. So I could create two regular expressions for extracting surname depending on this setting. If there is enough interest.

Actually there isn't a single setting. That would be nice for many reasons I have battled for in the past, but the reasons against such a thing are valid so I don't try any more

There are actually "lots" of settings, but none of them explicitly tell you what naming approach an author is using. If you read the settings carefully you will see this is the case - there is one to do with flipping names when adding, another to do with downloading metadata etc. These settings can be set completely independently and might not reflect "reality".

See the other threads with a search (within this forum was the most recent one) for all the reasons - really the biggest one is foreign names which don't tend to fit into a nice "firstname surname" bucket. Let alone all the issues of suffixes like Phd etc. It is a headache that we fudge our way around with complicated best guesses in the likes of the Find Duplicates and metadata download plugins.

[kacir]

Originally Posted by kiwidude

Actually there isn't a single setting. That would be nice for many reasons I have battled for in the past, but the reasons against such a thing are valid so I don't try any more

There is another solution ...
Let the user provide the string
search = ['author:"'+re.sub(r'(.*) ([^ ]*)',r'\2',a.strip().replace('|', ','))+'"' for a in authors.split(',')]
as a configuration option ;-)
Like you can define RE for book import or for "save to disk" or for plugboard.

You see, I had several reasons for creating this thread. Here they are, ordered by importance:

1. I was happy that I succeeded in creating my first meaningful private patch, and I wanted to share my joy with this wonderful community.

2. I wanted to share my hack with whomever might have the same itch that I was trying to scratch. With millions of Calibre users around the world, I am pretty sure there are [a few] users that might find this interesting.

3. I wanted to encourage people to try to have a peek at Calibre internals, because making a *small* patch wasn't nearly as difficult as I was expecting. Even if your Python programming experience is fairly limited.

4. I wanted to hear what others think about the usefulness of this (admittedly very specific) hack

5. I was thinking that it would be extremely cool to have a patch accepted into the main tree ;-).
But, now that I am up and running, I think that there will be more such little patches (especially where I can use RegularExpression to do something cool), and who knows ... perhaps one day ...