Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowing explicit 'method_name' on SerializerMethodField #6015

Closed
ablakey opened this issue Jun 4, 2018 · 3 comments
Closed

Allowing explicit 'method_name' on SerializerMethodField #6015

ablakey opened this issue Jun 4, 2018 · 3 comments

Comments

@ablakey
Copy link

ablakey commented Jun 4, 2018

I sat here trying to decide what's better etiquette: resurrecting a 3-year-old ticket or making a new one. Apologies if this is impolitely disruptive.

Relates to this discussion 3 years ago: #2420

An AssertionError is raised if you redundantly set 'method_name' to the same format as the method. eg:

type = SerializerMethodField('get_type')

def get_type(self, obj):
    return obj.type

The problem, in my opinion, is that it enforces one consistent style of "Django Magic" where you need to understand Django's implementation in order to understand the code. To a general Python reader, it's not clear that Django magically knows which method to call because the method name is similar to the field name.

The final say 3 years ago was: "...The behaviour as it currently stands is deliberate design and I'm happy with the benefit of enforcing a single consistent style." I don't think I understand what the benefit is. Enforced consistency, sure. But it's enforced implicitness.

Any change of opinion on this matter? I'd be happy to take responsibility of making the necessary changes if so. Thank you for your time.

@carltongibson
Copy link
Collaborator

Hi.

SerializerMethodField is no longer around. Instead all filters accept a method argument, allowing correct type validation at the form level.

Given that, I don’t think there’s an issue here.

@carltongibson
Copy link
Collaborator

Sorry, wrong repository.

@carltongibson
Copy link
Collaborator

The benefit is a lack of repetition.

Users were constantly writing lines like my_field = ... 'get_my_field' and nobody liked it. The 'get_my_field' was implied.

I can’t see any appetite for going back on that decision.

But no harm in asking. 🙂

kbussell added a commit to enefeq/django-rest-framework that referenced this issue Aug 4, 2018
Fixes encode#6015

Prevent an exception in `quantize()` when monkey-patching the decimal
library as cdecimal in Python 2 environments by always passing a valid
(not None) `rounding` value to `quantize()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants