Can you rewrite your summary in the imperitive mood, i.e., as if it were a command or order?
The following sentence should make sense if you substitute in your summary:
"This patch will <summary>"

dict.get accepts a default argument if the key is not present so this can be rewritten as:

lambda y: primary_widget_positions.get(y.widget_id, len(primary_widget_positions)


Also can we rename y to something like widget? y is not a good variable name.

Same as above.

This is a good opportunity to get comfortable with unit tests.
You'll need to write a unit test that sets up some state in the profile for the widget positions, performs a HTTP GET to the admin UI dashboard, and then checks the resulting template context to see if your fixes did what you'd expect them to do.
This will be done with Django's TestClient, which works like:

def test_blah(self):
    ...

    response = self.client.get(reverse('admin-dashboard'), ...)
    self.assertEqual(response.context['key'], expected_value)

Please run all tests, e.g., ./tests/runtests.py. They take a few minutes, but sometimes things cascade into other suites and break things.

In your description: 

`KeyError`


over

"KeyError"

We generally avoid \\ where we can:

total_initial_widgets = (
    len(response.context['selected_secondary_widgets']) +
    len(response.context['selected_primary_widgets']))

You can wrap this without the \\:


profile = Profile.objects.get_or_create(
    user=response.context['user'])[0]

blank line between these.

You are registering widgets outside of the try..finally.
Lets do it this way.

try:
    register_admin_widget(TestPrimaryWidget)
    register_admin_widget(TestSecondaryWIdget)

    # lines 298 thru 321 without the try/finally.

finally:
    try:
        unregister_admin_widget(TestPrimaryWidget)
    except KeyError:
        pass

    try:
        unregister_admin_widget(TestSecondaryWidget)
    except KeyError:
        pass


We need the nested try in the finally clause because soemthing could go wrong with registering either of the widgets and we would fall into the finally clause.

blank line between these.

We can pull out the selections into a intermediate variables to make things easier to read.

    primary_selections = profile.extra_data['primary_widget_selections']
    secondary_selections = profile.extra_data['secondaary_widget_selections']

    primary_selections[TestPrimaryWidget.widget_id] = '1'
    secondary_selections[TestSecondaryWidget.widget_id] = '1'

Again, we can wrap this without \ as above.

Blank line between expression and block statement.

This should actually be:

self.assertNotIn(TestPrimaryWidget, response.context['selected_primary_widgets'])


Otherwise the error message we would get when running tests (True is not False) is not helpful. This way, it will repr() print the contents of response.context['selected_primary_widgets'] to aid in debugging.

Blank line between block statements.

As per my comment above, this should actually be:

self.assertNotIn(TestSecondaryWidget, response.context['selected_secondary_widgets'])

While you're here, blank line between these.

Since we're doing the same operations essentially twice with different variables, maybe we should extract this into two functions so that we could do:

selected_primary_widgets, unselected_primary_widgets = _get_widget_selections(
    primary,
    profile_data.get('primary_widget_selections'))

sorted_primary_widgets = _sort_widgets(
    selected_primary_widgets,
    profile_data.get('primary_widget_positions'))

selected_secondary_widgets, unselected_secondary_widgets = _get_widget_selections(
    secondary_widgets,
    profile_data.get('secondary_widget_selections'))

sorted_secondary_widgets = _sort_widgets(
    selected_secondary_widgets,
    profile_data.get('secondary_widget_positions'))


where _get_widget_selections and _sort_widgets are new functions that encapsulate the current behaviour. This refactor would go a long way towards shortening the function bodies, encapsulating repeated logic, and making this code more readable.

Lets use widget over p here.

While you're here, blank line between these.

Lets use widget over s here.

While you're here, blank line between these.

This shouldn't need the parentheses around the lambda, should it?

While you're here, blank line between these.

This shouldn't need the parentheses around the lambda, should it?

Our imports should be alphabetically sorted so this (a) should go after the reviewboard.admin.validation imports and (b) needs its import( ... ) list to be sorted.

Comments should be full sentences and end in periods.

In this case just use \ to wrap to the next line since it saves a vertical line.

In this case just use \ to wrap to the next line since it saves a vertical line.

Missing Returns:

How about:

    A dictionary mapping widgets (:py:class:`~reviewboard.admin.widgets.
    Widget`) to whether or not they are selected (as a :py:class:`unicode`).

The summary should fit entirely on the first line.
How about Return widgets sorted based on their positions.

How about:

A dictionary mapping widget IDs to their ordinals.

Missing Returns:

After these tiny nits, I'm happy with this landing!

Uppercase before lowercase, so this would be the first import.

Single quotes.

Single quotes.

Single quotes.

Single quotes.

Can you add a comment here explaining why we're doing this? We both know why we wrap these in try..except and ignore the error, but future readers of this file might not grok it immediately.

Should be "admin/views.py"

This would be a little easier as:

profile = response.context['user'].get_profile()

This line can go away (the definition of a try block is to wrap something in case it fails).

This is a little bit too verbose (I can read the words "try", "pass", and "KeyError" in the code).
How about "If an error was encountered above, the widgets will not be registered. Ignore any errors here in that case"?

Add a blank line here, please.

We usually write the type as tuple, like this:
```

Returns:

    tuple of list:

    A 2-tuple containing a list of selected widgets (to display

    on the dashboard) and a list of the unselected widgets.

"""

Ship It!