Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

Point deserialize from string coordinates #10

Merged
merged 3 commits into from Jun 17, 2016
Merged

Point deserialize from string coordinates #10

merged 3 commits into from Jun 17, 2016

Conversation

lafrech
Copy link
Contributor

@lafrech lafrech commented Jun 16, 2016

This change allows Point to deserialize coordinates provided as strings rather than numbers (floats or ints).

I'm serving an API using webargs/flask and I'm using a NestedQueryFlaskParser as suggested in the docs.

I receive a query like this one:

http PUT "http://localhost:5000/buildings/57512e2227b613712d824fe9?name=building&position.x=100&position.y=50"

The custom parser generates a dictionary but does not deserialize the coordinate values into numbers.

I guess marshmallow-mongoengine's Point is the right place to do that. Or am I wrong?

@touilleMan, @kevin0571 any advice?

@coveralls
Copy link

coveralls commented Jun 16, 2016

Coverage Status

Coverage increased (+0.5%) to 94.517% when pulling 107d9a3 on Nobatek:point_deserialize_from_string_coordinates into 23b8f41 on touilleMan:master.

@touilleMan
Copy link
Owner

Given marshmallow already accept a string as number (https://github.com/marshmallow-code/marshmallow/blob/dev/marshmallow/fields.py#L612), I'm ok with this PR.

Just a single remark: could you add a test with a wrong string (something like data = {'point': { 'x': '10', 'y': '20foo' }})

I wait for this change, then merge&release a new version (unless you want to make other changes before that)

@coveralls
Copy link

coveralls commented Jun 17, 2016

Coverage Status

Coverage increased (+1.04%) to 95.039% when pulling 718ef62 on Nobatek:point_deserialize_from_string_coordinates into 23b8f41 on touilleMan:master.

@lafrech
Copy link
Contributor Author

lafrech commented Jun 17, 2016

Given marshmallow already accept a string as number (https://github.com/marshmallow-code/marshmallow/blob/dev/marshmallow/fields.py#L612), I'm ok with this PR.

Great.

Just a single remark: could you add a test with a wrong string (something like data = {'point': { 'x': '10', 'y': '20foo' }})

Done.

I wait for this change, then merge&release a new version (unless you want to make other changes before that)

Thanks !

(No, no other contrib planned at the moment.)

@lafrech
Copy link
Contributor Author

lafrech commented Jun 17, 2016

This is a corner case, as people would probably send such data as json, I suppose (ultimately, this is what I'm going to do, the query args method explained above was a test).

http PUT http://localhost:5000/buildings/57512e2227b613712d824fe9 name=building position:='{"x":100,"y":50}'

Anyway, casting to float can't do wrong.

@touilleMan touilleMan merged commit 545cd87 into touilleMan:master Jun 17, 2016
@touilleMan
Copy link
Owner

version 0.7.6 released ;-)

@lafrech lafrech deleted the point_deserialize_from_string_coordinates branch June 17, 2016 09:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants