Replace deepcopy of the Q object

[rrauenza]

No description provided.

[rrauenza]

Still trying to run unit tests locally, hoping marking this PR as ready would trigger some on project side.

[rrauenza]

See #542

[TomHaii]

Can also confirm that is indeed fixing the issue in deepcopying Q objects. (reproduced on Python 3.7)

[alphatownsman]

Is there some unittest can cover the case it fixes?

[rrauenza]

I think if the existing unit tests work, that is sufficient, since it shows the change doesn't break from the current method of copying.

We could assign a non picklable object attr to one of the children to prevent future regression?

[AdamDonna]

+1 to adding a non picklable object attr to prevent future regressions or even an explicit scenario that should break in 3.7 without this fix

[rrauenza]

@AdamDonna It's been a while -- I came back to try to add a test.

The test is successful, and it fails if I swap deepcopy back in. I'm not happy with the test data, but I could not figure out how to trigger the code path into django's trees.py.

Can we accept this PR so I can eventually remove my patch from my project?

(Also the pre commit failures look pre-existing..)

[rrauenza]

I am wondering, though, if this code is ok:

   def _copy_child(child):                                                               
        if isinstance(child, tuple):                                                      
            return child  # tuples are immutable, no need to make a copy.                 
        elif isinstance(child, Q):                                                        
            return _deepcopy_q_object(child)                                              
        else:                                                                             
            raise RuntimeError("Unknown child type: %s", type(child))       def _copy_child(child):                                                               
        if isinstance(child, tuple):                                                      
            return child  # tuples are immutable, no need to make a copy.                 
        elif isinstance(child, Q):                                                        
            return _deepcopy_q_object(child)                                              
        else:                                                                             
            raise RuntimeError("Unknown child type: %s", type(child))    

Yes, tuples are immutable.... but what they contain are not necessarilly, if, say it contained a dict.

[rrauenza]

Q() derives from tree.Node, which adds copy in Django 4.2+, and does this (from django source):

   def __copy__(self):
        obj = self.create(connector=self.connector, negated=self.negated)
        obj.children = self.children  # Don't [:] as .__init__() via .create() does.
        return obj

    copy = __copy__

Let's jump into the debugger and see what happens.

polymorphic/tests/test_admin.py .
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB set_trace (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> /home.local/rich/src/django-polymorphic/polymorphic/query_translate.py(99)_deepcopy_q_object()
-> obj = q.copy()
(Pdb) pp q
<Q: (OR: ('one2one__in', [<Model2D: id 1, field1 (CharField), field2 (CharField), field3 (CharField), field4 (CharField)>]))>
(Pdb) 
(Pdb) pp q.children
[('one2one__in',
  [<Model2D: id 1, field1 (CharField), field2 (CharField), field3 (CharField), field4 (CharField)>])]

Step past q.copy() and we get:

(Pdb) pp obj
<Q: (OR: ('one2one__in', [<Model2D: id 1, field1 (CharField), field2 (CharField), field3 (CharField), field4 (CharField)>]))>
(Pdb) pp obj.children
[('one2one__in',
  [<Model2D: id 1, field1 (CharField), field2 (CharField), field3 (CharField), field4 (CharField)>])]
(Pdb) 

Are these objects actually deep copied in django's copy()?

No. both the tuple and the Model are just references to the original. So my implementation for Django < 4.2 doesn't do anything different, IMHO.

(Pdb) id(obj.children[0][1][0])
140227184738432
(Pdb) id(q.children[0][1][0])
140227184738432
(Pdb) id(obj.children[0])
140364539161792
(Pdb) id(q.children[0])
140364539161792

[rrauenza]

There is one oddity, though, that I've highlighted in my latest push. Django's copy seems to change the case of some of the Q() parameters. See the comment in the code for more info.