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

`attribute` asymmetry #450

Closed
itajaja opened this Issue May 4, 2016 · 9 comments

Comments

Projects
None yet
4 participants
@itajaja

itajaja commented May 4, 2016

if I define a field like:

foo = fields.String(attribute="bar.baz")

then I can serialize {'bar': {'baz': 'myvalue'}} to {'bar': myvalue}. I'd expect this to be symmetric, but if I load I get to {'bar': myvalue} to {'bar.baz': 'myvalue'}. Is there a better way to flatten a nested value or am I missing something? This behavior is a little weird, is there a reason for this to be asymmetric?

@sloria sloria added the needs review label Mar 8, 2017

@sloria sloria added this to the 3.0 milestone Mar 20, 2017

@sloria

This comment has been minimized.

Member

sloria commented Mar 20, 2017

I agree this behavior is not intuitive. Looks like dot-delimited attribute syntax isn't being respected upon deserialization.

I would gladly review and merge a PR for this.

@sloria sloria added bug help wanted and removed needs review labels Mar 20, 2017

@sloria

This comment has been minimized.

Member

sloria commented Mar 20, 2017

I fixed this in 8d90fb9 . I'll release a hotfix shortly.

@sloria sloria closed this in 8d90fb9 Mar 20, 2017

@sloria

This comment has been minimized.

Member

sloria commented Mar 20, 2017

Fix is in 2.13.4 and 3.0.0b2.

@sloria sloria removed this from the 3.0 milestone Mar 20, 2017

@itajaja

This comment has been minimized.

itajaja commented Mar 20, 2017

Thanks! Keep up the great work :)

@density

This comment has been minimized.

density commented Mar 27, 2017

This change actually turned out to be backwards incompatible (it visibly changes the way deserialization works). Luckily, I have tests to catch things like this but for our friends who aren't so lucky, we should be more careful about minor point releases (this was released with version 2.13.4) with backwards incompatible changes...

@sloria

This comment has been minimized.

Member

sloria commented Mar 27, 2017

@density This was not considered a breaking change because it does not break any API contract (defined by documentation and tests) and goes against user expectation. Sorry for any inconvenience it may have caused you. We do take backwards-compatibility seriously, and we appreciate any feedback you can give in this regard.

@density

This comment has been minimized.

density commented Mar 28, 2017

No inconvenience caused!

@lafrech

This comment has been minimized.

Member

lafrech commented Mar 28, 2017

I can't help but quote this xkcd.

Workflow

@density

This comment has been minimized.

density commented Mar 28, 2017

fblackburn1 added a commit to wazo-pbx/xivo-confd that referenced this issue Jul 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment