Remove deprecations and add tests

[lucaswiman]

@bruth @naegelyd This pull request does some cleanup in preparation for the next final release started in #22.

Changes

• Corrected spelling of some comments using the scspell utility.
• Removed deprecation warnings and support for deprecated behavior.
• Updated docstrings to remove references to deprecations.
• Fixed an import path in a management command to reflect that only python 2.7+ is supported by modern versions of django.
• Added tests for several error conditions.
• Added tests for excluded_models.
• Started on additional tests for the symmetrical option to required_routes. I'm a little bit stuck here, since the behavior seems counterintuitive or wrong, and wanted to get advice on what the expected behavior is. (I verified that the test also fails on commits prior to Django 1.8+ support #22.)

[lucaswiman]

Without the expectedFailure, this test fails with the following error:

======================================================================
ERROR: test_symmetrical_required_route (tests.cases.core.tests.test_routes.FieldRouterTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/lucaswiman/opensource/modeltree/tests/cases/core/tests/test_routes.py", line 332, in test_symmetrical_required_route
    self.assertEqual([n.model for n in symmetric_tree._node_path(C)], [C])
  File "/Users/lucaswiman/opensource/modeltree/modeltree/tree.py", line 639, in _node_path
    model = self.get_model(model)
  File "/Users/lucaswiman/opensource/modeltree/modeltree/tree.py", line 381, in get_model
    .format(model_name))
ModelNotRelated: No model found named "<class 'tests.models.C'>"

----------------------------------------------------------------------

Given that there's a foreign key C->A, I don't see why setting symmetrical=True should make C unreachable. Even if the expected behavior is that it must include the C->D linkage, there's still a route via A->B->D->C. Am I correct in thinking this is incorrect behavior?

[bruth]

This route requires that a join occurs from C to D, so the path A->B->D->C would not work. However, I am not sure why this fails, since the symmetrical flag simply adds another route.

[naegelyd]

@lucaswiman @bruth Let's consider the case of this test. We have the following required routes:

'required_routes': [{
    'target': 'tests.D',
    'source': 'tests.C',
    'symmetrical': True,
}

This will result in required routes of the following being returned from _build_routes:

{
    (<class 'tests.models.D'>, <class 'tests.models.C'>): None, 
    (<class 'tests.models.C'>, <class 'tests.models.D'>): None
}

And here is our model tree:

#        A
#       / \
#      C   B
#      |  / \
#       D    G
#      / \   |
#     E   F  |
#      \ / \ |
#       J   H
#       |   |
#       K   I

Looking at those required routes and the model tree, we see we will never add nodes C or D because all the joins to them will be disallowed. A --> C, B-->D, F-->D, E-->D will all be rejected because of the symmetric routes in required route. We see that both C and D are targets in required routes so all those paths above will be rejected by join_allowed. Here is the relevant snippet:

        # Check if the join is allowed by a required rule
        for (_source, _target), _field in self._required_joins.items():
            if _target == target:
                if _source != source:
                    return False

As a result, neither C nor D will be added as nodes in the tree which explains the failure. The asymmetric case works fine because A-->C is allowed by join_allowed because C is not a target in any of the required routes. Given the explanation outlined above and the current behavior of join_allowed, I am not sure what model tree would include nodes that are symmetrical in required routes unless one of the nodes was the root node. Right now, using symmetrical requires you to go from node1-->node2 and node2-->node1 which is impossible. Hopefully the above is coherent. Haven't come up with a solution yet but wanted to outline my thoughts here for now.

[bruth]

Hmm. The behavior of symmetrical should be "when the target is D come from C" and "when the target is C come from D". It should not restrict the path to C or D, respectively, nor should it require both joins to be satisfied, since that is impossible.

[bruth]

Do you mind collapsing all of these imports into a single statement? Primarily to be consistent with the rest of the codebase.

[naegelyd]

Do you mind collapsing all of these imports into a single statement? Primarily to be consistent with the rest of the codebase.

@bruth Outside of collapsing the imports, is there anything left to do for this PR to be accepted? It would be great to get a release with these changes and those from #22.

[lucaswiman]

Sorry, I lost track of this. Yeah, I think the test needs to be updated based on @bruth's comments. I'll try to fix this up tomorrow and get it ready for a release.

[bruth]

Modeltree 2.0.0 was released a bit ago which supports Django 1.8+.