change SortedDict to OrderedDict + fix list and tuple concatination #2858
Conversation
|
Unfortunately the tests don't pass. |
|
Yes, sorry. I was not careful. Now all tests pass. |
|
@kazmiruk as a general note all compatibility checks should go in |
|
Yes, it makes sense. Done. |
| try: | ||
| from collections import OrderedDict | ||
| except ImportError: | ||
| from django.utils.datastructures import SortedDict as OrderedDict |
There was a problem hiding this comment.
Let's import these from compat instead.
|
Couple of inline comments, but otherwise this is looking good! |
| @@ -985,13 +992,16 @@ def restore_object(self, attrs, instance=None): | |||
| if field_name in attrs: | |||
| m2m_data[field_name] = attrs.pop(field_name) | |||
|
|
|||
| # Forward m2m relations | |||
| for field in meta.many_to_many + meta.virtual_fields: | |||
| def _inner_loop_code(field): | |||
There was a problem hiding this comment.
I'm confused about this - what's going on here, and should this really be part of the same pull request as the SortedDict/OrderedDict change?
|
Yes, I'm sorry that I didn't create separate branches for each fix. About meta.many_to_many + meta.virtual_fields in django 1.8 those properties has different type (list and tupple). This causes to exception when concatenates. |
|
I still think this should be two different pull requests, one for the compat changes and another for list/tuple concat changes. |
|
I think it's better just exclude list\tuple changes from this PR. #2857 about list\tuple changes |
|
Looks good - thanks! |
change SortedDict to OrderedDict + fix list and tuple concatination
I need to support django-rest-framework on django 1.8 a little bit more, but there are some errors and irritating warnings. This PR fixes error and changes deprecated SortedDict to OrderedDict (as I seen you are not using something special from SortedDict which not compatible with OrderedDict)