Changes between Initial Version and Version 1 of Ticket #19501, comment 4
- Timestamp:
- Mar 1, 2013, 4:40:24 PM (12 years ago)
Legend:
- Unmodified
- Added
- Removed
- Modified
-
Ticket #19501, comment 4
initial v1 1 1 I did some work on the patch. Custom fields using SubFieldBase do not use `__set__`, instead they use `__get__`. In addition I verified that nothing in Django relies on custom `__init__` or model `__init__` signals. Still, nothing in Django relies on `__setattr__`. 2 2 3 The easily most common action on load is to convert f rom DB format to Python format. But this is exactly what SubFieldBase is meant for. There might be other use cases, maybe something like calculating a value on `__init__`. This seems fragile - if you then go and change obj.somefield value and that field is part of the calculation your code will break. Thus, a property seems a lot better way to go for such cases.3 The easily most common action on load is to convert field value from DB format to Python format. But this is exactly what SubFieldBase is meant for. There might be other use cases, maybe something like calculating a value on `__init__`. This seems fragile - if you then go and change obj.somefield value and that field is used in the calculation your code will break. Thus, a property seems a lot better way for this particular use case. 4 4 5 Django's code base shows that one usually wants to differentiate `__init__` and load from DB, for example the init signals in Django act only when initializing object, but they do nothing for load from db case. Overriding `__init__` is also hard for this reason.5 Django's code base shows that one usually wants to differentiate `__init__` in Python and load from DB, for example the init signals in Django act only when initializing object, but they do nothing for load from db case. Overriding `__init__` is also hard for this reason, you don't know if the object is loaded from DB or if this is a "plain" init. 6 6 7 7 The performance numbers are nice: … … 20 20 }}} 21 21 22 If you add in the patch f rom removing the chunked iterators, then you get this:22 If you add in the patch for removing the chunked iterators (#18702), then you get this: 23 23 {{{ 24 24 Running 'query_all' benchmark ... … … 43 43 new = Empty() 44 44 new.__class__ = cls 45 new. = dict(zip(field_names, values))45 new.id, new.title = values 46 46 47 47 … … 55 55 Now, such a model will only work in read-only cases (no model._state), and wont work with .defer() or .only() (all fields must always be present), but if you happen to need speed for read-only case, you can get it. 56 56 57 The branches are here: https://github.com/akaariai/django/compare/fast_init_test and https://github.com/akaariai/django/compare/fast_init_and_nonchunked ( also the latest patch from #18702).57 The branches are here: https://github.com/akaariai/django/compare/fast_init_test and https://github.com/akaariai/django/compare/fast_init_and_nonchunked (with the latest patch from #18702 added). 58 58 59 Now, there surely is code that will break if this is committed as-is. So, I wonder if this should be opt-in or opt-out with model._meta flag, or if the whole load using `__init__` could be removed totally. 59 There surely is code that will break if this is committed as-is. So, I wonder if this should be opt-in or opt-out with model._meta flag, or if the whole load using `__init__` could be deprecated totally. 60 61 EDIT: Some typo fixes, some clarified wording...