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

Override fields not init #7

Closed
wants to merge 10 commits into from

Conversation

jtrain
Copy link
Collaborator

@jtrain jtrain commented Feb 7, 2017

Not sure, but it might be nicer to move field related filtering into the fields property rather than editing directly during __init__.

Jervis Whitley and others added 10 commits February 7, 2017 18:26
Prevent access to it, since it will be run and cached even when
serializer is initialized as nested.
Previously it would warn if they were present but empty.
__init__ is called when creating the serializer instance. It can be
during the request cycle or at django start time (nested serializer).

Instead do the fields filtering logic directly in the `fields` property.
Internal and external methods and properties use this public API for
working with fields. It should be safe to assume `fields` is only
accessed during the request cycle, so fetching `self.context` is
assumed safe.
This situation would occur if someone was accessing `fields` during
their own custom __init__ method.
I'm not convinced it is required.
I thought the code quality was good, but it requires docstrings on
everything.
@jtrain
Copy link
Collaborator Author

jtrain commented Feb 8, 2017

I've made changes (even to files I took from cookie-cutter) to keep the linter codacy happy.

It seems like it doesn't acknowledge # noqa comments. I don't think I'm going to make any more changes since the code passes my flake8 linter here.

@jtrain
Copy link
Collaborator Author

jtrain commented Mar 15, 2017

#8 duplicate of this.

@jtrain jtrain closed this Mar 15, 2017
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

Successfully merging this pull request may close these issues.

None yet

1 participant