From ddf0002bb760e5f1df664a81bd2a554c522f2c0f Mon Sep 17 00:00:00 2001 From: Nick Pope Date: Mon, 20 Sep 2021 23:34:24 +0100 Subject: [PATCH] Refs #32948 -- Renamed Node._new_instance() to Node.create(). Node._new_instance() was added in 6dd2b5468fa275d53aa60fdcaff8c28bdc5e9c25 to work around Q.__init__() having an incompatible signature with Node.__init__(). It was intended as a hook that could be overridden if subclasses needed to change the behaviour of instantiation of their specialised form of Node. In practice this doesn't ever seem to have been used for this purpose and there are very few calls to Node._new_instance() with other code, e.g. Node.__deepcopy__() calling Node and overriding __class__ as required. Rename this to Node.create() to make it a more "official" piece of private API that we can use to simplify a lot of other areas internally. The docstring and nearby comment have been reworded to read more clearly. --- django/db/models/sql/where.py | 2 +- django/utils/tree.py | 16 ++++++---------- tests/utils_tests/test_tree.py | 4 ++-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/django/db/models/sql/where.py b/django/db/models/sql/where.py index dbfd9a02a4..d3e08cc65e 100644 --- a/django/db/models/sql/where.py +++ b/django/db/models/sql/where.py @@ -171,7 +171,7 @@ class WhereNode(tree.Node): self.children[pos] = child.relabeled_clone(change_map) def clone(self): - clone = self.__class__._new_instance( + clone = self.__class__.create( children=None, connector=self.connector, negated=self.negated, diff --git a/django/utils/tree.py b/django/utils/tree.py index f67c90eae4..e82afe9bf7 100644 --- a/django/utils/tree.py +++ b/django/utils/tree.py @@ -25,17 +25,13 @@ class Node: self.connector = connector or self.default self.negated = negated - # Required because django.db.models.query_utils.Q. Q. __init__() is - # problematic, but it is a natural Node subclass in all other respects. @classmethod - def _new_instance(cls, children=None, connector=None, negated=False): + def create(cls, children=None, connector=None, negated=False): """ - Create a new instance of this class when new Nodes (or subclasses) are - needed in the internal code in this class. Normally, it just shadows - __init__(). However, subclasses with an __init__ signature that aren't - an extension of Node.__init__ might need to implement this method to - allow a Node to create a new instance of them (if they have any extra - setting up to do). + Create a new instance using Node() instead of __init__() as some + subclasses, e.g. django.db.models.query_utils.Q, may implement a custom + __init__() with a signature that conflicts with the one defined in + Node.__init__(). """ obj = Node(children, connector, negated) obj.__class__ = cls @@ -97,7 +93,7 @@ class Node: node other got squashed or not. """ if self.connector != conn_type: - obj = self._new_instance(self.children, self.connector, self.negated) + obj = self.create(self.children, self.connector, self.negated) self.connector = conn_type self.children = [obj, data] return data diff --git a/tests/utils_tests/test_tree.py b/tests/utils_tests/test_tree.py index 81002e68c9..04223964ba 100644 --- a/tests/utils_tests/test_tree.py +++ b/tests/utils_tests/test_tree.py @@ -69,11 +69,11 @@ class NodeTests(unittest.TestCase): self.node1.negate() self.assertFalse(self.node1.negated) - def test_new_instance(self): + def test_create(self): SubNode = type("SubNode", (Node,), {}) a = SubNode([SubNode(["a", "b"], OR), "c"], AND) - b = SubNode._new_instance(a.children, a.connector, a.negated) + b = SubNode.create(a.children, a.connector, a.negated) self.assertEqual(a, b) # Children lists are the same object, but equal. self.assertIsNot(a.children, b.children)