Colin, Thanks for your input. I was intending on rolling these changes into my composite fields work, since it would give a better illustration of why the changes were useful, but I can create a separate PR with the changes.
`get/set_instance_value` are perfectly valid names and I agree, slightly less redundant. `get_form_data` is not a good candidate, as these methods are used in places internal to the fields API where that name would make absolutely no sense whatsoever. Any broken code will be subject to a lengthy deprecation period using the old method names, obviously so I don't think it would cause much of a difference. I don't use form fields, so I can't share your pain. As far as `_get_cache_name` goes, I had initially written `subfield.name` to have the name relative to the composite field (so, having two composite fields of the same type would mean two different subfields with the same `name` attribute on the model). I've changed that implementation since (to support retrieving subfields via model._meta.get_field), so the change is no longer warranted. PR for the 'value_from_object' change will come later today though. Thanks, Thomas On 5 March 2015 at 15:14, Collin Anderson <[email protected]> wrote: > Hi, > > If it's not too much work, creating pull requests could help give some > context and show a bit more of the impact of your proposed changes. Some > days I think I'm better at reading Python than English :) > > value_from_object maybe should be value_from_instance or > get_instance_value, as in, pick this field's value off of the model > instance. "Field.get_field_value" sounds redundant. > > Re: value_from_object and save_form_data: looking at some code I wrote 5 > years ago that has mostly been untouched, renaming these would break my > code, but they are undocumented after all. I agree that the two go hand in > hand. I'd almost rather rename value_from_object to get_form_data, just so > it's clear what's going on. :) > While we're on the subject: it would be great if formfields also had a way > to override this getting and setting :) ("No, I want the _object_ not the > integer from the FK" :) > > Re: _get_cache_name(), using "name" makes more sense to me than "attname", > but I'm curious more about this "field.name not necessarily unique" > change :). > > Thanks, > Collin > > On Monday, March 2, 2015 at 12:37:37 AM UTC-5, Thomas Stephenson wrote: >> >> I was wondering whether there would be support for changes to some of the >> methods on the field API... >> >> 1. Rename `Field._get_val_from_obj` and `Field.value_from_object` methods >> >> These two methods do exactly the same thing (except one returns a default >> value if available). In addition, the 'value_from_object' is confusingly >> named, as it performs a completely different function than >> `value_to_string` and `to_python`, but is named similarly. >> >> Both of these methods should be deprecated and a new method >> `Field.get_field_value(model_instance, use_default=True)` should be >> added. If `use_default` is provided and `True`, then the implementation >> should redirect to `field.get_default()`, otherwise an AttributeError would >> be raised. >> >> 2. Rename `Field.save_form_data` >> >> `Field.save_form_data()` should be renamed to `Field.get_field_value()` >> for symmetry with `Field.get_field_value()`. >> >> 3. `Field._get_cache_name()` and should use field.attname. >> >> I'm planning to submit a PR in the next couple of days that makes ` >> field.name` not necessarily unique. All non-hidden fields will still be >> unique, so it won't cause a problem with the `model._meta.get_field()` API, >> but it will cause problems if fields can overwrite each others cached >> values. This should not be a breaking change, since _get_cache_name is an >> implementation detail. >> > -- > You received this message because you are subscribed to the Google Groups > "Django developers (Contributions to Django itself)" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/django-developers. > To view this discussion on the web visit > https://groups.google.com/d/msgid/django-developers/6d6bad97-647c-4a8c-ba83-b567b9094d82%40googlegroups.com > <https://groups.google.com/d/msgid/django-developers/6d6bad97-647c-4a8c-ba83-b567b9094d82%40googlegroups.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/django-developers. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CA%2Bm8oA_9we%2BZ6nJ1biB9EisJUDJeEOGZe_LgOimnpoHZLS8%3DEQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
