-
Notifications
You must be signed in to change notification settings - Fork 64
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
Download #1050
Download #1050
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment about geography tables. Let's be sure the downloaded geometry comes in a format we can then save in the CSV and work with.
|
||
def _download_query(project, dataset, table, limit=None, offset=None): | ||
full_table_name = '`{}.{}.{}`'.format(project, dataset, table) | ||
query = 'SELECT * FROM {}'.format(full_table_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will use this for both downloading a plain dataset or a geographies dataset. I'd like to be sure of the geometry format downloaded and written in the CSV.
Could we add a test to make sure that we can read/upload to CARTO the geometry in the downloaded CSV file for a geography table? (or any other type of test that you think it might be interesting for this case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The geometry is saved as WKT as string in the file
"POLYGON((-4.22608307660106 43.3126868301895, ...))"
We could use a carto-do-public-data
for that, but I would not really like to make a tests end to end against big query and carto.
(still thinking about it)
f8643de
to
09d64f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice implementation.
Added some suggestions, the one about the notebook I'd like to be addressed 🙏
Co-Authored-By: Alberto Romeu <alrocar@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 🦉
Acceptance 🍏 Let's address my last comment (about ™️ ) and this is ready to merge. |
558617f
to
c43c434
Compare
Solves https://github.com/CartoDB/data-observatory/issues/189