-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add ignore_fields option #80
Conversation
Co-Authored-By: Jonas Krüger Svensson <jonas-ks@hotmail.com>
@vbabiy , any status on this one? I'm upvoting it atleast, would be a nice feature to have. 👍 |
@JonasKs sorry, I am going to test and merge today. |
@vbabiy , no rush, just asking :) |
That'd be a highly desirable feature indeed, any update on this? |
if key not in ignore_fields and new_key not in ignore_fields: | ||
new_dict[new_key] = camelize(value, **options) | ||
else: | ||
new_dict[new_key] = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be:
new_dict[key] = value
instead?
Otherwise, the new key (post transformation) is applied, which is not what is expected.
For example, using __typename
in ignore_fields
won't work here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about something like this instead?
def camelize (data, **options):
ignore_fields = options.get('ignore_fields') or ()
if isinstance(data, Promise):
data = force_text(data)
if isinstance(data, dict):
if isinstance(data, ReturnDict):
new_dict = ReturnDict(serializer = data.serializer)
else:
new_dict = OrderedDict()
for (key, value) in data.items():
if isinstance(key, Promise):
key = force_text(key)
if key in ignore_fields:
new_dict[key] = camelize(value, **options)
else:
if isinstance(key, str) and ('_' in key):
new_key = re.sub(camelize_re, underscore_to_camel, key)
else:
new_key = key
new_dict[new_key] = camelize(value, **options)
return new_dict
if is_iterable(data) and not isinstance(data, str):
return [camelize(item, **options) for item in data]
return data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be:
new_dict[key] = valueinstead?
Otherwise, the new key (post transformation) is applied, which is not what is expected.
For example, using
__typename
inignore_fields
won't work here.
Thanks for your comment! It could be that the naming is a bit confusing. What we really are doing is ignoring fields under the given key. In other words, we don't want to recursively modify any of the data under the specified key. We might need to split the option of not changing specific key names (like __typename
in your example) with not modifying any data under certain key names (like extraData
or extra_data
in my example, where the field name should be changed but the others shouldn't).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am okay with this being called ignore_fields
if we want to support the other model maybe call it ignore_keys
We have a use-case where we need to avoid camelizing/underscorizing certain field data. For example:
is underscorized as:
Instead, by adding
extra_data
orextraData
toignore_fields
, we can now get the expected behavior of underscorizing/camelizing:This relates to another PR that did this by passing the option to the constructor, but here we use the settings file to configure the option.