Skip to content
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

Overriding Field.__deepcopy__ for RegexField #2617

Closed
wants to merge 1 commit into from
Closed

Overriding Field.__deepcopy__ for RegexField #2617

wants to merge 1 commit into from

Conversation

kchang
Copy link
Contributor

@kchang kchang commented Feb 27, 2015

Problem:

Serializers are deepcopying a declared RegexField[source], but it'll error out with the message. TypeError: cannot deepcopy this pattern object.

screen shot 2015-02-27 at 2 12 06 pm
screen shot 2015-02-27 at 2 12 52 pm
screen shot 2015-02-27 at 2 58 45 pm

Compiled regex pattern cannot be deepcopied. The overridden
__deepcopy__ mimics the behavior as the parent method, but
checks for `regex` instead of `validators`
@thedrow
Copy link
Contributor

thedrow commented Jun 23, 2015

I think this needs a test in order to verify that now the RegexField can be deep copied with compiled regex patterns.

Something like:

# ...
serializer = TestSerializer(data={'regex_field': 'abc'})
try:
  serializer.get_fields()
except TypeError as e:
  if e.msg == 'cannot deepcopy this pattern object' and isinstance(e, TypeError):
    assert False, 'Compiled regex patterns cannot be deep copied.'
  else:
    traceback.print_traceback()
    raise e

args = self._args[:1] + copy.deepcopy(self._args[1:])
kwargs = dict(self._kwargs)

# deepcopy doesn't work on compiled regex pattern
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if it'd instead be simpler to handle all this logic alongside the similar 'validators' special casing in Field.__deepcopy__?

@ruiwen
Copy link

ruiwen commented Sep 8, 2016

Hi there,

Wondering if there's been a decision made on this? I'm running into the same deepcopy issue with RegexFields.

Having regex taken out of kwargs

    def __init__(self, regex, **kwargs):
        super(RegexField, self).__init__(**kwargs)
        <..snip..>

doesn't seem to avoid them being in self._kwargs

 def __new__(cls, *args, **kwargs):
        """
        When a field is instantiated, we store the arguments that were used,
        so that we can present a helpful representation of the object.
        """
        <..snip..>
        instance._kwargs = kwargs
        return instance

and it's self._kwargs that actually ends up being deepcopy()ed in __deepcopy__

    def __deepcopy__(self, memo):
        """
        When cloning fields we instantiate using the arguments it was
        originally created with, rather than copying the complete state.
        """
        args = copy.deepcopy(self._args)
        kwargs = dict(self._kwargs)
        <..snip..>
        kwargs = copy.deepcopy(kwargs)
        <..snip..>

@tomchristie tomchristie added this to the 3.4.7 Release milestone Sep 8, 2016
@tomchristie
Copy link
Member

Bumping up the priority to ensure this gets reviewed sometime over the coming weeks.

@ruiwen
Copy link

ruiwen commented Sep 8, 2016

To add on, this happens only when I'm using a compiled regex object as the regex argument (either as a positional parameter, or as a keyword argument).

Using a pattern string works fine

eg.

import re
USERNAME_RE = re.compile(r"[a-z0-9]+")

username = serializers.RegexField(regex=USERNAME_RE)  # This eventually fails

username = seriazliers.RegexField(regex=USERNAME_RE.pattern)  # This works

@tomchristie
Copy link
Member

Noted. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants