Conversation
Rather than subclass a `Field` and override `__init__` you may want a more data-driven approach where the defaults for `read_only`, `write_only` and `allow_null` are specified on the class.
e.g.
```python
class ReadOnlyLink(serializers.HyperlinkedRelatedField)
read_only = True
```
|
build failed because of unrelated installation problem. |
|
This is certainly less contentious than before - although I'm still somewhat conflicted. The declarative style is absolutely more graceful, but we're also introducing extra state which makes it more complicated to determine the behavior (eg you need to figure out if There's also some consideration to be given to if we should be using There's also a potential conflict here with #2473 which would suggest that for any api settings based stuff that we probably shouldn't be declaring them as class attributes (which we currently do) which would run in the other direction to the style here. I'm not sure what the right answer is here. One thing that it does help clarify to me is that It looks like we should probably be using explicit keyword arguments through (ie stop using kwargs.pop, and always list extra allowable keyword arguments in the init, as a form of declaration.) |
|
Closing this off in it's current state - too wide ranging. Am sympathetic to the intent, but if anyone wants to pull things in this direction I'd suggest by starting with a pull request with a smaller change footprint. For example make this change for max_length/min_length/max_value/min_value values only, and leave everything else as-is. That'd make for a much more easily reviewable PR, and easier to properly discuss. |
2 participants
Rather than subclass a Field and override init you may want a more data-driven approach where the defaults for read_only, write_only and allow_null are specified on the class.
This is same as #2254 but without the extra commit that is gone to the PR #2469