Aggregate thanks across notes for course, sort by accumulated course thanks

[btbonval]

This should be sort of like aggregating number of notes per school, where each school has a number of courses and the courses have a number of notes. That was a one-liner to find dynamically: https://github.com/FinalsClub/karmaworld/issues/218#issuecomment-30990944

Although it is cached for-loop in other places: https://github.com/FinalsClub/karmaworld/blob/dda166347b2e53e9e12e7204325f120765210c7b/karmaworld/apps/courses/models.py#L75

Probably easier to do this dynamically.

[btbonval]

Actually neither of those are good options, because this information must be aggregated together with the rest of the information for each individual course. It makes more sense to implement a Course method which reaches into the notes when called.

[btbonval]

This is too slow.

It would be better to do the one liner to fetch all the note aggregated thanks per course at once time, then in the template load the thanks out of that separate fetch. Not sure how to extract from a dictionary in the template the way I need.

In python it would look like this:

context['thanks'] = aggregation
context['thanks'][course.id]

In the template it'd be more like:

thanks.course.id

Except that it'd try to evaluate thanks[course][id], not thanks[course.id].

[btbonval]

http://stackoverflow.com/questions/1070398/how-to-set-a-value-of-a-variable-inside-a-template-code https://docs.djangoproject.com/en/1.5/ref/templates/builtins/#with

something like

{% with thank_key=course.id %}
{{ course_thanks.thank_key }}
{% endwith %}

[btbonval]

aggregate and annotate both return lists. The former a list of dicts, the latter a list of objects.

It'd be ideal if the dictionary was thanks[course_id] = thanks, coming straight out of Django. If that isn't possible, rework aggregate output into the expected dictionary. That latter option might end up in no speed savings.

[btbonval]

parse the list of dictionaries into a generator of 2-tuples which is executed as a dictionary conversion. this uses the bare metal Python stuff, so it should go faster than for loops?

dict(((x['id'], x['value']) for x in list_of_dicts))

[btbonval]

https://docs.djangoproject.com/en/1.5/ref/templates/api/#variables-and-lookups "Note that “bar” in a template expression like {{ foo.bar }} will be interpreted as a literal string and not using the value of the variable “bar”, if one exists in the template context."

No tips on how to toss a variable into a dict in a template? Maybe I can call the dictionary's get() method and pass in the variable key as an argument.

[btbonval]

http://stackoverflow.com/a/1333277/1867779 Make a filter that takes the arguments. I'll make a dict.get() filter.

The premade filters that take arguments are almost all string and list filters. Not a single filter for dictionaries that I could find already existing.

[btbonval]

Just to get a baseline I reloaded the beta site, and suddenly it was loading on my count of about 2 seconds (probably faster than that), which was far faster than it had been loading when I first installed the fix for this ticket.

I added the speed enhancement. It didn't seem significantly faster, nor did it seem at all slower. The code was already written so I gave it a shot on beta, but beta seemed to have hastened prior to the push.

I'm tempted to remove it as it is confusing and doesn't add appreciable value.

[btbonval]

Fresh firefox browser. Using the network analysis tool of the web dev bar. Profiling:

• with optimization, refresh button
  • 28 requests, 1.38 seconds
  • 28 requests, 1.37 seconds
  • 28 requests, 1.66 seconds
• without optimization, refresh
  • 28 requests, 1.38 seconds
  • 28 requests, 1.4 seconds
  • 28 requests, 1.46 seconds
• with optimization code, new window
  • 31 requests, 2.0 seconds
  • 32 requests, 1.7 seconds
  • 32 requests, 2.0 seconds
• without optimization, new window
  • 32 requests, 1.98 seconds
  • 32 requests, 2.18 seconds
  • 32 requests, 1.94 seconds

There is no way this optimization is different with any statistical significance. The super long load times I saw on beta were a fluke, and this optimization adds nothing but confusion to the code.

Reverting optimization.

[btbonval]

optimization reverted in commit 1cd804f934d6d4b12ca8f43d898fe0f1e4a2b880

It didn't add any value.

Beta seems to be running fast enough now.