Skip to content

Make ReturnDict support dict union operators on Python 3.9 and later #8302

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

Merged
merged 1 commit into from
Dec 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions rest_framework/utils/serializer_helpers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import sys
from collections import OrderedDict
from collections.abc import Mapping, MutableMapping

Expand Down Expand Up @@ -28,6 +29,22 @@ def __reduce__(self):
# but preserve the raw data.
return (dict, (dict(self),))

if sys.version_info >= (3, 9):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for the version gate here?
The methods could be always defined, even if only python >= 3.9 uses them.

Copy link
Contributor Author

@spookylukey spookylukey Dec 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not a case of only Python 3.9 using them - no matter what your Python version, if you define those methods, then | will do dictionary merging like it does in 3.9 (I have confirmed this). So the question is - if I'm on Python 3.8, what do I expect these would do:

>>> {} | {}
>>> serializer_instance.data | {}

On Python 3.8, the first one will produce TypeError: unsupported operand type(s) for |: 'dict' and 'dict' and there is nothing we can do about that. So I expect most people would expect the second to do that as well.

I think we are asking for confusion and trouble if we effectively backport PEP 584 to earlier Python versions, but only for the return value of Serializer.data, and nowhere else (because we can't).

# These are basically copied from OrderedDict, with `serializer` added.
def __or__(self, other):
if not isinstance(other, dict):
return NotImplemented
new = self.__class__(self, serializer=self.serializer)
new.update(other)
return new

def __ror__(self, other):
if not isinstance(other, dict):
return NotImplemented
new = self.__class__(other, serializer=self.serializer)
new.update(self)
return new


class ReturnList(list):
"""
Expand Down
22 changes: 22 additions & 0 deletions tests/test_serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -740,3 +740,25 @@ class TestSerializer(A, B):
'f4': serializers.CharField,
'f5': serializers.CharField,
}


class Test8301Regression:
@pytest.mark.skipif(
sys.version_info < (3, 9),
reason="dictionary union operator requires Python 3.9 or higher",
)
def test_ReturnDict_merging(self):
# Serializer.data returns ReturnDict, this is essentially a test for that.

class TestSerializer(serializers.Serializer):
char = serializers.CharField()

s = TestSerializer(data={'char': 'x'})
assert s.is_valid()
assert s.data | {} == {'char': 'x'}
assert s.data | {'other': 'y'} == {'char': 'x', 'other': 'y'}
assert {} | s.data == {'char': 'x'}
assert {'other': 'y'} | s.data == {'char': 'x', 'other': 'y'}

assert (s.data | {}).__class__ == s.data.__class__
assert ({} | s.data).__class__ == s.data.__class__