CartoDB / cartoframes

CARTO Python package for data scientists
BSD 3-Clause "New" or "Revised" License
251 stars 63 forks source link

Add cartodbfy step for create_table_from_query and copy_table functions #1744

Closed Mmoncadaisla closed 3 years ago

Mmoncadaisla commented 3 years ago

Context

After this PR, the cartodbfication step for to_carto was removed from the ContextManager's _drop_create_table_from_query method to be performed later on this function instead.

However, this has introduced a bug, causing that both create_table_from_query and copy_table functions (which relied on this step for cartodbfying tables) result in non-cartodbfied tables.

Further context can be found here: https://app.clubhouse.io/cartoteam/story/175987/economicalinsurance-cartoframes-datasets-generated-from-query-are-not-cartodbfyed

Relevant PR changes

shortcut-integration[bot] commented 3 years ago

This pull request has been linked to Clubhouse Story #175987: [economicalinsurance] CARTOframes datasets generated from query are not CartoDBfyed.

Mmoncadaisla commented 3 years ago

@Jesus89 I've just added a test to verify the cartodbfication step is executed when calling the ContextManager's create_table_from_query method, please let me know if you think it's ok and/or if there is any other additional test we should add as part of this PR 🙏🏼

Jesus89 commented 3 years ago

I would add also tests without the cartodbfy parameter, to check that the default value is properly set in all the functions

Mmoncadaisla commented 3 years ago

Thank you @Jesus89 (sorry I had misread it at first!) added tests to verify that the input default cartodbfy parameter is True in copy_table, create_table_from_query and ContextManager's create_table_from_query 🙂