Changes between Initial Version and Version 1 of Ticket #19501, comment 4


Ignore:
Timestamp:
Mar 1, 2013, 4:40:24 PM (11 years ago)
Author:
Anssi Kääriäinen

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #19501, comment 4

    initial v1  
    11I 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__`.
    22
    3 The easily most common action on load is to convert 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 part of the calculation your code will break. Thus, a property seems a lot better way to go for such cases.
     3The 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.
    44
    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.
     5Django'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.
    66
    77The performance numbers are nice:
     
    2020}}}
    2121
    22 If you add in the patch from removing the chunked iterators, then you get this:
     22If you add in the patch for removing the chunked iterators (#18702), then you get this:
    2323{{{
    2424Running 'query_all' benchmark ...
     
    4343    new = Empty()
    4444    new.__class__ = cls
    45     new. = dict(zip(field_names, values))
     45    new.id, new.title = values
    4646
    4747
     
    5555Now, 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.
    5656
    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).
     57The 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).
    5858
    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.
     59There 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
     61EDIT: Some typo fixes, some clarified wording...
Back to Top