8000 Fix near lat lon for REST client by slobdell · Pull Request #216 · twilio/twilio-python · GitHub
[go: up one dir, main page]
Skip to content

Fix near lat lon for REST client #216

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Fix near lat lon for REST client #216

wants to merge 1 commit into from

Conversation

slobdell
Copy link

the "near_lat_long" parameter doesn't actually work for the search() method. The API takes a string like "42.0046,-93.2140" but it currently passes NearLatLong=42.0046&NearLatLong=-93.2140 as GET parameters

@slobdell
Copy link
Author

I was going to add test coverage, but the whole test_rest.py was set to inactive?

@skimbrel
Copy link
Contributor

It should work if you generate the string yourself. The argument in favor of taking a list of two strings makes sense to me, though, so this seems reasonable. It'll need to be backwards-compatible though, which means detecting whether the parameter given for near_lat_long is a string and passing it straight through as is.

Yeah, test_rest.py is inactive, but there are more tests in tests/test_available_phonenumber.py that you could add to. Please make sure to add a test that exercises the backwards-compatible path by passing a string like "37.7833,-122.4167".

@slobdell
Copy link
Author

sure

I suppose the much easier solution then would be to just update the doc string so that "near_lat_long" took a formatted string.

@skimbrel
Copy link
Contributor

Yep, seems reasonable. I'll get that done.

@carlosdp carlosdp force-pushed the master branch 3 times, most recently from 614259c to 2b69150 Compare September 23, 2015 00:21
carlosdp pushed a commit that referenced this pull request Nov 2, 2015
Change near_lat_long to be formatted string, fixes #216
e13a621
@carlosdp carlosdp mentioned this pull request Nov 2, 2015
Merged
@carlosdp carlosdp closed this in #231 Nov 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@slobdell @skimbrel
0