#4027 closed New feature (fixed)
Document how to make copies of model instances
Reported by: | Marek Kubica | Owned by: | Tomek Paczkowski |
---|---|---|---|
Component: | Documentation | Version: | dev |
Severity: | Normal | Keywords: | db, copy |
Cc: | pythonmailing@…, simon@…, Forest Bond, mmitar@…, tomek+django@…, bruth@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Although it does not seem documented anywhere (but I think, it should, as it is handy) model instances can be copied by setting the primary key to None
in which case they get a new ID and get saved as new object into the database preserving their old values. Unfortunately, this does not apply to ManyToMany
fields of the model: they have to be saved before setting the primary key to None
(ModelInstance.manytomanyfield.all()
), then the instance has to be saved and afterwards they can be added to the new object as it has again a valid ID and saved for the second time. It seems to me that saving an object twice to circumvent this is somewhat hackish.
The most important thing to me is that copying objects should work in a clean way (setting the primary key to None
is more or less clean enough, provided that it is documented somewhere and works as expected). Such a possibility is needed sometimes: I have to maintain a large number of objecs which often differ only in one field, so I can simply copy the old one and change the value.
Attachments (4)
Change History (30)
comment:1 by , 18 years ago
Summary: | Copying does not copy ManyToMany field values → ability to make a copies of model instances |
---|---|
Triage Stage: | Unreviewed → Design decision needed |
comment:2 by , 18 years ago
Thanks Gary, this is the same thing I heard on IRC. An official method for copying is exactly what I need. Fine, I'll start a discussion on this on django-dev.
comment:3 by , 18 years ago
I've posted to django-developers now, the discussion can be found in Gogle Groups.
comment:4 by , 18 years ago
Triage Stage: | Design decision needed → Accepted |
---|
Many-to-many fields can only be added and saved after the models they are referring to have been saved (or, at least, have a primary key). That cannot be avoided. So it sounds like you need to write an external copy() function that handles this.
Seems like a reasonable thing to do, so feel free to knock up a patch. A method on the Model class wouldn't be insane.
comment:5 by , 18 years ago
Has patch: | set |
---|---|
Needs documentation: | set |
Needs tests: | set |
Patch needs improvement: | set |
I wrote a first version of the patch: it implements copy()
as method of the Model
class defined in base.py
. It was tested by a small helper script - I'm not sure, but it probably needs some unittests, too. As this was my first patch submitted to Django, I was not sure how to implement it correctly, so this patch is more of a draft that is free to discussion. If you have any comments on it - just post, I'll try to improve this patch to bring it into a stage where it is ready for checkin.
comment:6 by , 17 years ago
Cc: | added |
---|
comment:7 by , 17 years ago
Triage Stage: | Accepted → Design decision needed |
---|
I was unable to use this patch, because it implicitly save()s the newly copied instance.
To me, plain old copy() should only copy the attributes.
Perhaps some arguments (eg commit=True), and/or maybe using deepcopy() to include the manytomany's would be a better approach?
comment:8 by , 16 years ago
With QSRF and model inheritance, setting the pk of a subclass model to None is no longer enough to make a copy. From what I can tell, the best working approach seems to be:
def copy_model_instance(obj): from django.db.models import AutoField initial = dict([(f.name, getattr(obj, f.name)) for f in obj._meta.fields if not isinstance(f, AutoField) and\ not f in obj._meta.parents.values()]) return obj.__class__(**initial)
This is without handling m2m relationships.
by , 15 years ago
Attachment: | django-model-copying.diff added |
---|
New patch implementing Model.copy with recursion. Includes tests.
comment:9 by , 15 years ago
Hi,
Attached is a new patch that implements Model.copy with recursion. The patch includes tests (although the tests probably miss some corner cases).
Let me know what you think.
Thanks,
Forest
comment:10 by , 15 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
Reset a "Needs tests" and "Patch needs improvement" flags pending review of latest patch.
comment:11 by , 15 years ago
Model inheritance makes this more difficult since the inheriting classes uses a base pointer that references the base model. Setting the id to None and calling save() does not create a new instance (at least it doesn't in my case on Django 1.0.2).
comment:12 by , 14 years ago
Cc: | added |
---|
Hi Luke,
My patch does not rely on setting self.id to None.
I will try to bring this ticket up on the mailing list. Perhaps it can be fixed in 1.3.
Thanks,
Forest
comment:13 by , 14 years ago
Cc: | added |
---|
comment:14 by , 14 years ago
Component: | Database layer (models, ORM) → Documentation |
---|---|
Has patch: | unset |
Triage Stage: | Design decision needed → Accepted |
This is possible in a few lines of code for each model. You can copy many-to-many field by doing new_instance.m2m = old_instance.m2m.all()
, for example. Ditto for foreign keys. This isn't always want you want when creating a copy of a model, however (think deep-copy versus shallow-copy), so really has to be left up to the individual case. For any particular model, a duplicate method is short enough already that Django doesn't need additions.
*However*, a documentation patch to explain this little fragment would be useful. We should start capturing some of these little patterns.
comment:15 by , 14 years ago
Hi Malcolm,
It sounds like your suggestion is to manually copy each field from the old instance to the new one. Why not provide a method to automate this?
Also, my patch implements recursion (deep copying). This is not a very straight-forward thing to do, so it seems like having one implementation that works in Django core would be useful.
I realize my patch does more than what the ticket reporter was initially looking for. Would you prefer that I create a new ticket to cover this additional functionality? I personally do need support for deep copying.
Thanks,
Forest
comment:16 by , 14 years ago
I also think it would be useful to have a best-practice method on object which covers this (or few of them for different kinds of cloning, all nicely documented) so that programmer does not need to care about internals of the objects (for example, what if objects will represent non-relational data someday, will m2m field be there, will there be something else to also copy?).
comment:17 by , 14 years ago
Summary: | ability to make a copies of model instances → Document how to make copies of model instances |
---|
In response to Forest and mitar: let's keep this a documentation ticket as Malcolm suggested, and move any deep/shallow copy feature additions to a new ticket (and probably start a mailing list discussion to make sure we have consensus on the feature).
For the time being, a ticket this old really needs some kind of resolution, and documenting a simple and useful pattern that covers 90% of cases is a good idea, even if copy methods do get implemented at a later date.
I would say that either docs/topics/db/models or docs/topics/db/queries is probably the place to put the docs for this. Despite the fact that topics/db/models is a more appropriate section, it actually fits better context-wise with topics/db/models near the sections of updating and deleting objects. A case for going into either one could be made, and ultimately there should probably be a cross-ref from one section to the other whichever way one it ends up in.
comment:18 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:19 by , 13 years ago
Easy pickings: | unset |
---|---|
Owner: | changed from | to
UI/UX: | unset |
by , 13 years ago
Attachment: | 4027-copy-model-docs.diff added |
---|
(Git) diff with documentation update describing copying model instances
comment:20 by , 13 years ago
Cc: | added |
---|---|
Status: | new → assigned |
I've written short section on copying model instances. I surely need some help with people that know how to write.
comment:21 by , 13 years ago
Has patch: | set |
---|---|
Needs documentation: | unset |
by , 13 years ago
Attachment: | 4027-copy-model-docs-text.diff added |
---|
Same as last patch, tiny language corrections added.
comment:22 by , 13 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
I've made a few tiny language improvements. Otherwise, it looks good to me, with clear and simple examples. RFCed.
comment:23 by , 13 years ago
Cc: | added |
---|
I tackled this problem as a separate app: https://github.com/cbmi/django-forkit. It is simply an abstract model that implements a few methods such as fork
, reset
, and diff
. It handles all three relationship types, though there are a few known issues (e.g. M2M through models are not yet supported). Deep diff
ing is not yet fully baked, and the reset
method must propagate that a reset
is intended for all related objects (rather than creating new ones). Saving related objects are deferred until the whole hierarchy has been traversed to ensure data integrity (and provides ways of altering the objects post fork
/reset
).
Here are the tests for it thus far: https://github.com/cbmi/django-forkit/blob/master/forkit/tests/models.py. I would be interested in hearing feedback on a few of the assumptions I made (e.g. which relationships to choose for a shallow copy). And definitely send a pull request or submit a ticket if there are more elegant ways to make use of the Django model internals.
I am posting here only to make others aware of it, but I don't think it is appropriate to go into Django proper.
Since Django has no official method for copying model instances, I am going to change this ticket's title to reflect that; otherwise, this ticket should probably be marked invalid. Marek, you might want to start a discussion on the django-dev mailing list about this topic.