From 4aa32c8e4d0fa9e5c2d443be7d3f1348c98f2694 Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Mon, 23 Nov 2015 16:20:05 +0100 Subject: [PATCH 1/4] Fix GSTs to show the source categories of the items consolidated into the name, and to correct the average rating during consolidation --- src/calibre/db/categories.py | 25 ++++++++++++++++++++++--- src/calibre/gui2/tag_browser/model.py | 5 ++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/calibre/db/categories.py b/src/calibre/db/categories.py index 631d84a96b..0b0e86ac1d 100644 --- a/src/calibre/db/categories.py +++ b/src/calibre/db/categories.py @@ -21,11 +21,12 @@ class Tag(object): __slots__ = ('name', 'original_name', 'id', 'count', 'state', 'is_hierarchical', 'is_editable', 'is_searchable', 'id_set', 'avg_rating', 'sort', - 'use_sort_as_name', 'category', 'search_expression') + 'use_sort_as_name', 'category', 'search_expression', 'original_categories') def __init__(self, name, id=None, count=0, state=0, avg=0, sort=None, category=None, id_set=None, search_expression=None, - is_editable=True, is_searchable=True, use_sort_as_name=False): + is_editable=True, is_searchable=True, use_sort_as_name=False, + original_categories=None): self.name = self.original_name = name self.id = id self.count = count @@ -39,6 +40,7 @@ class Tag(object): self.use_sort_as_name = use_sort_as_name self.category = category self.search_expression = search_expression + self.original_categories = None def __unicode__(self): return u'%s:%s:%s:%s:%s'%(self.name, self.count, self.id, self.state, self.category) @@ -198,12 +200,29 @@ def get_categories(dbcache, sort='name', book_ids=None, first_letter_sort=False) if user_cat_is_gst: # for gst items, make copy and consolidate the tags by name. if n in names_seen: + # We must combine this node into a previous one with + # the same name ignoring case. As part of the process, + # remember the source categories and correct the + # average rating t = names_seen[n] other_tag = taglist[label][n] t.id_set |= other_tag.id_set t.count += other_tag.count + t.original_categories.add(other_tag.category) + + total_rating = 0 + count = 0 + for id_ in t.id_set: + rating = book_rating_map[id_] + if rating: + total_rating += rating/2 + count += 1 + if total_rating and count: + t.avg_rating = total_rating/count else: - t = copy.copy(taglist[label][n]) + # Must deepcopy so we don't share the id_set between nodes + t = copy.deepcopy(taglist[label][n]) + t.original_categories = {t.category} names_seen[n] = t items.append(t) else: diff --git a/src/calibre/gui2/tag_browser/model.py b/src/calibre/gui2/tag_browser/model.py index d0d3922cea..f35436e2e9 100644 --- a/src/calibre/gui2/tag_browser/model.py +++ b/src/calibre/gui2/tag_browser/model.py @@ -184,7 +184,10 @@ class TagTreeItem(object): # {{{ return self.icon_state_map[tag.state] if role == Qt.ToolTipRole: tt = [self.tooltip] if self.tooltip else [] - tt.append('%s:%s' % (tag.category, tag.original_name)) + if tag.original_categories: + tt.append('%s:%s' % (','.join(tag.original_categories), tag.original_name)) + else: + tt.append('%s:%s' % (tag.category, tag.original_name)) ar = self.average_rating if ar: tt.append(_('Average rating for books in this category: %.1f') % ar) From b4efa96292f21adcc32ab9043ff54d5dd06ab6db Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Mon, 23 Nov 2015 16:20:53 +0100 Subject: [PATCH 2/4] Only put a node into intermediate_nodes if it is involved in a hierarchy --- src/calibre/gui2/tag_browser/model.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/calibre/gui2/tag_browser/model.py b/src/calibre/gui2/tag_browser/model.py index f35436e2e9..1f4a662eef 100644 --- a/src/calibre/gui2/tag_browser/model.py +++ b/src/calibre/gui2/tag_browser/model.py @@ -594,7 +594,6 @@ class TagsModel(QAbstractItemModel): # {{{ n = self.create_node(parent=node_parent, data=tag, tooltip=tt, icon_map=self.icon_state_map) category_child_map[tag.name, tag.category] = n - intermediate_nodes[tag.original_name, tag.category] = tag else: for i,comp in enumerate(components): if i == 0: @@ -606,10 +605,11 @@ class TagsModel(QAbstractItemModel): # {{{ if t.type != TagTreeItem.CATEGORY]) if (comp,tag.category) in child_map: node_parent = child_map[(comp,tag.category)] - node_parent.tag.is_hierarchical = \ - '5state' if tag.category != 'search' else '3state' - if tag.id_set is not None and node_parent.tag.id_set is not None: - node_parent.tag.id_set |= tag.id_set + t = node_parent.tag + t.is_hierarchical = '5state' if tag.category != 'search' else '3state' + if tag.id_set is not None and t.id_set is not None: + t.id_set |= tag.id_set + intermediate_nodes[t.original_name, t.category] = t else: if i < len(components)-1: original_name = '.'.join(components[:i+1]) From 99adf52046bacfcc861c2f110a0108dc6764c4f9 Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Mon, 23 Nov 2015 16:21:11 +0100 Subject: [PATCH 3/4] Cache the computed average rating --- src/calibre/gui2/tag_browser/model.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/calibre/gui2/tag_browser/model.py b/src/calibre/gui2/tag_browser/model.py index 1f4a662eef..e95b516151 100644 --- a/src/calibre/gui2/tag_browser/model.py +++ b/src/calibre/gui2/tag_browser/model.py @@ -73,6 +73,7 @@ class TagTreeItem(object): # {{{ is_searchable=category_key not in ['search']) elif self.type == self.TAG: self.tag = data + self.cached_average_rating = None self.tooltip = tooltip or '' @@ -119,6 +120,8 @@ class TagTreeItem(object): # {{{ return self.tag.avg_rating if not self.children: return self.tag.avg_rating # leaf node, avg_rating is correct + if self.cached_average_rating: + return self.cached_average_rating total = num = 0 for child in self.children: r = child.average_rating @@ -129,7 +132,8 @@ class TagTreeItem(object): # {{{ total += 1 num += self.tag.avg_rating try: - return num/float(total) + self.cached_average_rating = num/float(total) + return self.cached_average_rating except ZeroDivisionError: return 0 From 8ff0bf848110ebe2965f208f308176964ff2eb97 Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Mon, 23 Nov 2015 16:21:27 +0100 Subject: [PATCH 4/4] Fix counting items. The old way misstated the counts for user categories because it was using the cumulative id_set from the original category. --- src/calibre/db/categories.py | 2 +- src/calibre/gui2/tag_browser/model.py | 24 ++++++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/calibre/db/categories.py b/src/calibre/db/categories.py index 0b0e86ac1d..37138895f0 100644 --- a/src/calibre/db/categories.py +++ b/src/calibre/db/categories.py @@ -207,7 +207,7 @@ def get_categories(dbcache, sort='name', book_ids=None, first_letter_sort=False) t = names_seen[n] other_tag = taglist[label][n] t.id_set |= other_tag.id_set - t.count += other_tag.count + t.count = len(t.id_set) t.original_categories.add(other_tag.category) total_rating = 0 diff --git a/src/calibre/gui2/tag_browser/model.py b/src/calibre/gui2/tag_browser/model.py index e95b516151..b8f6da34fc 100644 --- a/src/calibre/gui2/tag_browser/model.py +++ b/src/calibre/gui2/tag_browser/model.py @@ -74,6 +74,7 @@ class TagTreeItem(object): # {{{ elif self.type == self.TAG: self.tag = data self.cached_average_rating = None + self.cached_item_count = None self.tooltip = tooltip or '' @@ -137,6 +138,21 @@ class TagTreeItem(object): # {{{ except ZeroDivisionError: return 0 + @property + def item_count(self): + if not self.tag.is_hierarchical or len(self.children) == 0: + return self.tag.count + if self.cached_item_count: + return self.cached_item_count + + def child_item_set(node): + s = set() | node.tag.id_set # force a copy of the set + for child in node.children: + s |= child_item_set(child) + return s + self.cached_item_count = len(child_item_set(self)) + return self.cached_item_count + def data(self, role): if role == Qt.UserRole: return self @@ -174,8 +190,7 @@ class TagTreeItem(object): # {{{ else: name = tag.name if role == Qt.DisplayRole: - count = len(tag.id_set) - count = count if count > 0 else tag.count + count = self.item_count if count == 0: return ('%s'%(name)) else: @@ -217,8 +232,7 @@ class TagTreeItem(object): # {{{ name = tag.original_name else: name = tag.name - count = len(tag.id_set) - count = count if count > 0 else tag.count + count = self.item_count rating = self.average_rating if rating: rating = ',rating=%.1f' % rating @@ -611,8 +625,6 @@ class TagsModel(QAbstractItemModel): # {{{ node_parent = child_map[(comp,tag.category)] t = node_parent.tag t.is_hierarchical = '5state' if tag.category != 'search' else '3state' - if tag.id_set is not None and t.id_set is not None: - t.id_set |= tag.id_set intermediate_nodes[t.original_name, t.category] = t else: if i < len(components)-1: