Skip to content
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

URL Updates and Google Warning #79

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cksammons7
Copy link

Hello!

I've been wanting to use the cesiumpy package, but have had problems similar to the 'Issues'. Following the cesiumjs tutorial, I made some changes to the cesiumpy code and after these changes, the package now successfully works. As mentioned in the git commits, I updated the URLs, the divid, and the geocoder.

When I ran the package the first time, there would be a Google warning. So I changed the geocoder to one that did not require a Key API for the map. I thought this would fix the issue, but still nothing would happen after running the code. I then looked at the html cesiumpy produces, and noticed differences compared to the html from the cesiumjs tutorial. So I made additional changes to match the html code.

I'm new to GitHub and coding, so I wasn't sure what to include in this but hopefully it covers what I updated. Any pointers for the future would be greatly appreciated.

Thank you!

@pep8speaks
Copy link

Hello @cksammons7! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 105:80: E501 line too long (117 > 79 characters)
Line 106:80: E501 line too long (135 > 79 characters)

@@ -9,7 +9,7 @@
import cesiumpy.util.common as com

# ToDo: want different geocoders?
_GEOCODER = GoogleV3()
_GEOCODER = GeocodeFarm()
Copy link

@opt12 opt12 Jul 23, 2020

Choose a reason for hiding this comment

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

This gives me a

NameError: name 'GeocodeFarm' is not defined

Add the import to the beginning (line 7)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants