8000 add namedetails argument to Nominatim by enriqueav · Pull Request #368 · geopy/geopy · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

add namedetails argument to Nominatim #368

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
Merged
merged 3 commits into from
Aug 4, 2019
Merged

add namedetails argument to Nominatim #368

merged 3 commits into from
Aug 4, 2019

Conversation

enriqueav
Copy link
Contributor

Add namedetails argument to nominatim #367.

I needed to simplify the logic of the function because flake8 was giving error.

$ flake8 geopy
geopy/geocoders/osm.py:210:5: C901 'Nominatim.geocode' is too complex (24)

If you can think about another option to workaround this issue of complexity, please do let me know.

@enriqueav enriqueav mentioned this pull request Aug 1, 2019
Closed
KostyaEsmukov
KostyaEsmukov requested changes Aug 3, 2019 8000
View reviewed changes
Copy link
Member
@KostyaEsmukov KostyaEsmukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, besides the country_codes change this looks good to me.

Regarding the flake8 error: just raise the limit, it's the most reasonable way to deal with this problem for now.

geopy/setup.cfg

Line 5 in da82a4a

max-complexity = 23

@KostyaEsmukov
Copy link
Member

I have just verified that the added test passes for OpenMapQuest and PickPoint as well, so this is good to go as soon as the country_codes change would be reverted.

@enriqueav
Copy link
Contributor Author

Thanks for the feedback and the explanation.
I have reverted the change and just increased the max complexity.
8000

KostyaEsmukov
KostyaEsmukov approved these changes Aug 4, 2019
View reviewed changes
Copy link
Member
@KostyaEsmukov KostyaEsmukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@KostyaEsmukov KostyaEsmukov merged commit 85ccae7 into geopy:master Aug 4, 2019
@enriqueav enriqueav changed the title add namedetails argument to Nominatim, simplify country_codes logic add namedetails argument to Nominatim Aug 5, 2019
@enriqueav enriqueav deleted the add_namedetails_nominatim branch August 5, 2019 04:22
@k-solnushkin k-solnushkin mentioned this pull request Aug 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KostyaEsmukov KostyaEsmukov KostyaEsmukov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3D32
Development

Successfully merging this pull request may close these issues.
2 participants
@enriqueav @KostyaEsmukov
0