Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KeyError on similar_objects #80

Closed
karanbhangui opened this issue Sep 2, 2011 · 14 comments
Closed

KeyError on similar_objects #80

karanbhangui opened this issue Sep 2, 2011 · 14 comments
Assignees
Milestone

Comments

@karanbhangui
Copy link

Traceback:
File "/usr/local/lib/python2.7/dist-packages/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/usr/local/lib/python2.7/dist-packages/django/contrib/auth/decorators.py" in _wrapped_view
  23.                 return view_func(request, *args, **kwargs)
File "/home/ubuntu/web/notewagon-django/notewagon/views.py" in view_course
  120.     related = course.tags.similar_objects()
File "/usr/local/lib/python2.7/dist-packages/taggit/utils.py" in inner
  125.         return func(self, *args, **kwargs)
File "/usr/local/lib/python2.7/dist-packages/taggit/managers.py" in similar_objects
  230.                 tuple(result[k] for k in lookup_keys)

Exception Type: KeyError at /s/university-of-waterloo/fdsg1yl7dk/math-115-linear-algebra-eng
Exception Value: (35L, 1755L)

The steps to the trace above are as follows:

  1. import/scrape a bunch of Resource items (I used youtube videos) and create tags based on the tags in the video
  2. add some tags to a Course ("Linear Algebra", vectors, eigenvalues, etc)
  3. observe error
@karanbhangui
Copy link
Author

any thoughts on this issue?

@macropin
Copy link

I've hit the same issue.

@thomaspurchas
Copy link

I believe that this is a Django bug. Look at ticket #19149.

The quickest way to fix this is to move the TaggableManager on to the subclass.

@vhf
Copy link

vhf commented Sep 17, 2013

I also have hit the same issue.

I found a dirty workaround by limiting the number of objects returned by similar_objects(). In taggit/managers.py, in similar_objects(), slicing the queryset after qs.order_by('-n) seems to solve the issue.

@The-Fonz
Copy link

I am experiencing the same error with django-taggit 0.21.3. The workaround doesn't work for me, it gives the same issue. Is there any other fix?

@gelbander
Copy link

@The-Fonz Are you using SQLite? I'm having the same issue using sqlite (even with the workaround) but there is no issue with postgres.

@The-Fonz
Copy link

@gelbander Yes I am, thanks for the tip, will try postgres!

@The-Fonz
Copy link

I switched to postgres but still had the KeyError, fixed it by changing line 260 in managers.py from items[(getattr(obj, remote_field.field_name),)] = obj to items[(obj.id,)] = obj, so now line 275 can properly access the dict. I'm using Wagtail but I'm not deep enough into Wagtail/Django/ModelCluster/Taggit to see why this bug appears. I implemented the fix by subclassing _ClusterTaggableManager and ClusterTaggableManager.

@return1
Copy link

return1 commented Nov 8, 2017

i am having the same issue with @django-taggit 0.22.1@ using Wagtail CMS

@strindhaug
Copy link

Still having the same issue with django-taggit 0.24.0 and wagtail 2.5.1 almost 8 years later...
Any progress on this?

The workaround that The-Fonz suggested still works, but it's pretty annoying to implement as I have to copy/paste a rather large method just to change one line of it and every time there is a change to the related files I have to diff the original code to update it.


If anyone's interested, here's my current working drop in replacement for ClusterTaggableManager implementing The-Fonz's workaround:

# taggablemanager.py
from __future__ import unicode_literals
from modelcluster.contrib.taggit import _ClusterTaggableManager, ClusterTaggableManager
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.utils import six
from taggit.utils import require_instance_manager

# All this crap is mostly boilerplate and copy and pasted code just to fix one line from taggit/managers.py


class _FixedClusterTaggableManager(_ClusterTaggableManager):
    @require_instance_manager
    def similar_objects(self):
        lookup_kwargs = self._lookup_kwargs()
        lookup_keys = sorted(lookup_kwargs)
        qs = self.through.objects.values(*six.iterkeys(lookup_kwargs))
        qs = qs.annotate(n=models.Count("pk"))
        qs = qs.exclude(**lookup_kwargs)
        qs = qs.filter(tag__in=self.all())
        qs = qs.order_by("-n")

        # TODO: This all feels like a bit of a hack.
        items = {}
        if len(lookup_keys) == 1:
            # Can we do this without a second query by using a select_related()
            # somehow?
            f = self.through._meta.get_field(lookup_keys[0])
            remote_field = f.remote_field
            rel_model = remote_field.model
            objs = rel_model._default_manager.filter(
                **{
                    "%s__in"
                    % remote_field.field_name: [r["content_object"] for r in qs]
                }
            )
            for obj in objs:
                # THE FOLLOWING 2 LINES IS THE ACTUAL FIX:
                # items[(getattr(obj, remote_field.field_name),)] = obj
                items[(obj.id,)] = obj
        else:
            preload = {}
            for result in qs:
                preload.setdefault(result["content_type"], set())
                preload[result["content_type"]].add(result["object_id"])

            for ct, obj_ids in preload.items():
                ct = ContentType.objects.get_for_id(ct)
                for obj in ct.model_class()._default_manager.filter(pk__in=obj_ids):
                    items[(ct.pk, obj.pk)] = obj

        results = []
        for result in qs:
            obj = items[tuple(result[k] for k in lookup_keys)]
            obj.similar_tags = result["n"]
            results.append(obj)
        return results


class FixedClusterTaggableManager(ClusterTaggableManager):
    def __get__(self, instance, model):
        # override TaggableManager's requirement for instance to have a primary key
        # before we can access its tags
        manager = _FixedClusterTaggableManager(
            through=self.through, model=model, instance=instance, prefetch_cache_name=self.name
        )

        return manager

Assuming this file is in the same directory as the models.py where you want to use a ClusterTaggableManager you can import it like so without changing anything else:

# from modelcluster.contrib.taggit import ClusterTaggableManager
from .taggablemanager import FixedClusterTaggableManager as ClusterTaggableManager 

@auvipy auvipy self-assigned this May 8, 2019
@auvipy auvipy added this to the 1.2.0 milestone May 8, 2019
@eon01
Copy link

eon01 commented Feb 17, 2020

It seems that this bug isn't fixed in version 1.2.0.

@codystevelarson
Copy link

Here is a workaround for those not wanting to eject and override the package:

In this scenario we use taggit to tag multiple objects (tools and toolboxes) so the "tt.object_id = ct.id and tt.content_type_id in (select dct.id from django_content_type dct where dct.app_label = 'core' and dct.model = 'tool')" line is to ensure we find the correct object id, in this case it is just for tools. This admittedly is not the best solution but better than nothing. The function only needs the instance of the object you want to find similar items for, in our case a tool queried from Tool.objects.get(pk=id). It returns a list of similar tools ordered by the most matches then by most recently added.

def get_similar_tools(tool):
    tags = tool.tags.all().values_list('id', flat=True)
​
    similar_tools = Tool.objects.raw(
        """
        select x.id, count(x.id) matching_tags from (select ct.id, count(tt.tag_id) as tag_count
        from core_tool ct 
        join taggit_taggeditem tt on tt.object_id  = ct.id and tt.content_type_id in 
                (select dct.id from  django_content_type dct where dct.app_label = 'core' and dct.model = 'tool') 
        join taggit_tag t on t.id = tt.tag_id 
        where t.id in %s and ct.id != %s
        group by (ct.id, tt.tag_id)
        order by ct.id) x
        group by x.id
        order by matching_tags desc, x.id desc
        """, params=[tuple(tags), tool.id])
​
    similar_tool_ids = []
    for similar_tool in similar_tools:
        similar_tool_ids.append(similar_tool.id)
​
    sts = list(Tool.objects.filter(pk__in=similar_tool_ids))
    sts.sort(key=lambda t: similar_tool_ids.index(t.pk))
​
    return sts

@czr137
Copy link
Contributor

czr137 commented Mar 1, 2021

Many wagtail users have reported this issue over the years. @jdufresne, as you seem to be the last active maintainer of this project, what needs to happen to put this and #508 and #424 to rest, and how can I help to make that happen?

@auvipy
Copy link
Contributor

auvipy commented Mar 18, 2021

thanks @czr137

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests