-
Notifications
You must be signed in to change notification settings - Fork 63
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
Use DODataset to download a dataset from DO #1594
Use DODataset to download a dataset from DO #1594
Conversation
…o-dataframe-and-to
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.
LGTM, but I'd take advantage to remove old code.
…o-dataframe-and-to
…d-to' of github.com:CartoDB/cartoframes into dgaubert/ch61421/integrate-do-client-in-to-dataframe-and-to
@rafatower, maybe we want to remove the class |
Ok, by removing unused method I need to make the ultimate sacrifice. 😥 |
Yeah, sure, if the class is not used anymore, delete it and related tests! |
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.
👍
Off-topic: the DO repository objects are the ones we should replace in CF's to grab from DO API instead of old rails endpoints. I'll create an issue for that.
Please, I need an early review.
Still pending changes in
carto-python
to addlimit
andorder_by
.