Feature/fix enrichment new catalogue #1083

alejandrohall · 2019-10-09T20:27:28Z

No description provided.

…oframes into feature/fix_enrichment_new_catalogue

This reverts commit 8bb6da9.

…/fix_enrichment_new_catalogue

alrocar · 2019-10-10T12:24:05Z

@alejandrohall there are some tests failing, could you take a look?

…/fix_enrichment_new_catalogue

alejandrohall · 2019-10-15T03:23:17Z

@alrocar Done!

simon-contreras-deel

It looks perfect, but I would change dataset names used in tests using fake ones.

simon-contreras-deel · 2019-10-15T07:13:27Z

test/data/enrichment/test_service.py

        FROM `carto-do-customers.{user_dataset}\
-            .ags_demographics_crimerisk_usa_blockgroup_2015_yearly_2018` enrichment_table
+            .view_ags_demographics_crimerisk_usa_blockgroup_2015_yearly_2018` enrichment_table


I think we are trying to avoid using real names

We need real names because of functions are using the real catalog, so we need real examples of table names. Also, I cannot see any problems, because we are offering publicly this dataset through website and catalog

cartoframes/data/enrichment/polygons_enrichment.py

elenatorro · 2019-10-18T09:51:55Z

cartoframes/data/enrichment/enrichment_service.py

@@ -162,14 +170,6 @@ def __process_agg_operators(agg_operators, variables):
    return agg_operators_result


__process_agg_operators method should also take into account what happens if this argument is a string, as we're doing in the enrich_polygons method. If it's a string, it throws 'str' object has no attribute 'copy'

elenatorro · 2019-10-18T10:49:31Z

I've added a couple of comments. Although I made some changes to make the enrich_polygons method work without errors locally, the variables are not being added in the result dataframe. Therefore, we need to fix these issues before merging this PR.

elenatorro · 2019-10-18T12:47:16Z

cartoframes/data/enrichment/points_enrichment.py

-                   variables_underscored='_'.join(variables), enrichment_table=table,
-                   enrichment_geo_table=table_to_geotable[table], user_dataset=user_dataset,
-                   working_project=working_project, data_table=data_table,
+        '''.format(enrichment_id=enrichment_id, variables_underscored='_'.join(variables),


variables_underscored is not being used

Removed unused variable from format method!

Jesus89 · 2019-10-18T14:48:05Z

Let's take into account this too https://github.com/CartoDB/data-observatory/issues/188#issuecomment-540737126, before merging the PR

…/fix_enrichment_new_catalogue

…riable

…chment

elenatorro · 2019-10-22T14:50:16Z

I discovered another error that is related with the enrich_points method: the aggregation column returns Decimal values. When reading a source, we've a method called encode_geodataframe that raises the following error: Object of type Decimal is not JSON serializable

If we convert the column type to float, it works. My question is: should we return the agg column always with float type? or is this something we should change when encoding the geodataframe?

cc @alejandrohall @Jesus89

Jesus89 · 2019-10-23T08:27:44Z

I would add this fix (convert to float) in the lib.

Jesus89 · 2019-10-23T09:39:26Z

For some reason, after merging the enriched DataFrame in the main DataFrame the pandas method to_json does not work anymore. However, the to_json method works in both DataFrames separately. So it's still a mistery why after merging both DataFrames, an object type is converted to Decimal.

Fortunately, there is a solution. The fix consists of using a custom JSONEncoder for the to_json method when we convert a DF into a GDF.

…toDB/cartoframes into feature/fix_enrichment_new_catalogue

elenatorro · 2019-10-23T11:08:36Z

cartoframes/utils/utils.py

 def encode_geodataframe(data):
    filtered_geometries = _filter_null_geometries(data)
-    data = _set_time_cols_epoc(filtered_geometries).to_json()
+    data = _set_time_cols_epoc(filtered_geometries).to_json(cls=CustomJSONEncoder)


elenatorro

LGTM 🚀

Alejandro Hall added 8 commits October 9, 2019 17:46

Get geographies from metadata

81a8491

Merge branch 'feature/1038-nested-filters' of github.com:CartoDB/cart…

9c822d7

…oframes into feature/fix_enrichment_new_catalogue

Merge branch 'feature/1038-nested-filters' of github.com:CartoDB/cart…

825bba1

…oframes into feature/fix_enrichment_new_catalogue

Add external table to query

acaea5a

Add alias in variables to enrich

8bb6da9

Revert "Add alias in variables to enrich"

9e6491a

This reverts commit 8bb6da9.

Multiple fixes

2137acb

Merge branch 'develop' of github.com:CartoDB/cartoframes into feature…

1a30f24

…/fix_enrichment_new_catalogue

alejandrohall added the enrichment label Oct 9, 2019

alejandrohall requested review from simon-contreras-deel and alrocar October 9, 2019 20:27

Import catalog class well

55f392d

alrocar assigned alejandrohall Oct 10, 2019

Alejandro Hall added 7 commits October 14, 2019 22:06

Merge branch 'develop' of github.com:CartoDB/cartoframes into feature…

782ea31

…/fix_enrichment_new_catalogue

Fix test

c127691

Fix test

ab9c05e

Fix test

17f79a5

Fix test

3796850

Fix test

f5c71bc

Fix test

ab02cdd

simon-contreras-deel reviewed Oct 15, 2019

View reviewed changes

Alejandro Hall added 2 commits October 15, 2019 13:11

Remove unused module

58df0b5

Rename catalog dataset

f7d54d0

simon-contreras-deel unassigned alejandrohall Oct 18, 2019

simon-contreras-deel approved these changes Oct 18, 2019

View reviewed changes

elenatorro suggested changes Oct 18, 2019

View reviewed changes

elenatorro assigned alejandrohall Oct 18, 2019

elenatorro reviewed Oct 18, 2019

View reviewed changes

elenatorro mentioned this pull request Oct 18, 2019

[Doc - Guides] Add Quickstart Guide #966

Closed

Alejandro Hall added 3 commits October 21, 2019 13:31

Merge branch 'develop' of github.com:CartoDB/cartoframes into feature…

b7da8ba

…/fix_enrichment_new_catalogue

Use variable get_list feature instead making one request for every va…

32b1545

…riable

Fix bugs regarding polygons enrichment and enhacements in points enri…

23e50ea

…chment

Extract imports in decode_geometry

fec43d9

elenatorro and others added 5 commits October 23, 2019 11:51

Add information about DO 2.0

b645461

Rearrange args

752847d

Use CustomJSONDecoder to convert a DataFrame

57aee4b

Improve legend exceptions

41372bf

Merge branch 'feature/fix_enrichment_new_catalogue' of github.com:Car…

3155cf8

…toDB/cartoframes into feature/fix_enrichment_new_catalogue

Jesus89 force-pushed the feature/fix_enrichment_new_catalogue branch from bd9d001 to 3155cf8 Compare October 23, 2019 11:00

Jesus89 assigned Jesus89 and elenatorro Oct 23, 2019

elenatorro reviewed Oct 23, 2019

View reviewed changes

elenatorro approved these changes Oct 23, 2019

View reviewed changes

Jesus89 merged commit 74dc81b into develop Oct 23, 2019

Jesus89 deleted the feature/fix_enrichment_new_catalogue branch October 23, 2019 11:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/fix enrichment new catalogue #1083

Feature/fix enrichment new catalogue #1083

alejandrohall commented Oct 9, 2019

alrocar commented Oct 10, 2019

alejandrohall commented Oct 15, 2019 •

edited

Loading

simon-contreras-deel left a comment

simon-contreras-deel Oct 15, 2019

alejandrohall Oct 15, 2019

elenatorro Oct 18, 2019 •

edited

Loading

alejandrohall Oct 22, 2019

elenatorro commented Oct 18, 2019

elenatorro Oct 18, 2019

alejandrohall Oct 22, 2019

Jesus89 commented Oct 18, 2019

elenatorro commented Oct 22, 2019

Jesus89 commented Oct 23, 2019

Jesus89 commented Oct 23, 2019

elenatorro Oct 23, 2019

elenatorro left a comment

		@@ -162,14 +170,6 @@ def __process_agg_operators(agg_operators, variables):
		return agg_operators_result

Feature/fix enrichment new catalogue #1083

Feature/fix enrichment new catalogue #1083

Conversation

alejandrohall commented Oct 9, 2019

alrocar commented Oct 10, 2019

alejandrohall commented Oct 15, 2019 • edited Loading

simon-contreras-deel left a comment

Choose a reason for hiding this comment

simon-contreras-deel Oct 15, 2019

Choose a reason for hiding this comment

alejandrohall Oct 15, 2019

Choose a reason for hiding this comment

elenatorro Oct 18, 2019 • edited Loading

Choose a reason for hiding this comment

alejandrohall Oct 22, 2019

Choose a reason for hiding this comment

elenatorro commented Oct 18, 2019

elenatorro Oct 18, 2019

Choose a reason for hiding this comment

alejandrohall Oct 22, 2019

Choose a reason for hiding this comment

Jesus89 commented Oct 18, 2019

elenatorro commented Oct 22, 2019

Jesus89 commented Oct 23, 2019

Jesus89 commented Oct 23, 2019

elenatorro Oct 23, 2019

Choose a reason for hiding this comment

elenatorro left a comment

Choose a reason for hiding this comment

alejandrohall commented Oct 15, 2019 •

edited

Loading

elenatorro Oct 18, 2019 •

edited

Loading