From f8099f1831f0200ab402c13a056554eb7979fca4 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Fri, 21 Apr 2023 21:28:04 -0700 Subject: [PATCH 1/5] add more informative error handling to geocode --- parsons/geocode/census_geocoder.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/parsons/geocode/census_geocoder.py b/parsons/geocode/census_geocoder.py index 43ec3e691a..be643a33b2 100644 --- a/parsons/geocode/census_geocoder.py +++ b/parsons/geocode/census_geocoder.py @@ -90,12 +90,11 @@ def geocode_address_batch(self, table): :widths: 40 :header-rows: 1 - * - Column Data - * - Unique ID - * - Street - * - City - * - State - * - Zipcode + * - Column Names + * - street + * - city + * - state + * - zip `Args:` table: Parsons Table @@ -105,12 +104,18 @@ def geocode_address_batch(self, table): """ logger.info(f"Geocoding {table.num_rows} records.") + if set(table.columns) != {"street", "city", "state", "zip"}: + msg = "Table must ONLY include `['street', 'city', 'state', 'zip']` as columns." + \ + "Tip: try using `table.cut()`" + raise ValueError(msg) + chunked_tables = table.chunk(BATCH_SIZE) batch_count = 1 records_processed = 0 geocoded_tbl = Table([[]]) for tbl in chunked_tables: + geocoded_tbl.concat(Table(petl.fromdicts(self.cg.addressbatch(tbl)))) records_processed += tbl.num_rows logger.info(f"{records_processed} of {table.num_rows} records processed.") From 0e088163e3a07a4b1454c2ea794f1f01cd400ef8 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Fri, 21 Apr 2023 21:59:55 -0700 Subject: [PATCH 2/5] fix(geocoder): Include id as required --- parsons/geocode/census_geocoder.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/parsons/geocode/census_geocoder.py b/parsons/geocode/census_geocoder.py index be643a33b2..f474893640 100644 --- a/parsons/geocode/census_geocoder.py +++ b/parsons/geocode/census_geocoder.py @@ -91,6 +91,7 @@ def geocode_address_batch(self, table): :header-rows: 1 * - Column Names + * - id (must be unique) * - street * - city * - state @@ -105,8 +106,8 @@ def geocode_address_batch(self, table): logger.info(f"Geocoding {table.num_rows} records.") if set(table.columns) != {"street", "city", "state", "zip"}: - msg = "Table must ONLY include `['street', 'city', 'state', 'zip']` as columns." + \ - "Tip: try using `table.cut()`" + msg = "Table must ONLY include `['id', 'street', 'city', 'state', 'zip']` as" + \ + "columns. Tip: try using `table.cut()`" raise ValueError(msg) chunked_tables = table.chunk(BATCH_SIZE) From 1bae3b45f6e57ac39e5e48ae3dc793448271edd7 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Mon, 24 Apr 2023 10:23:56 -0700 Subject: [PATCH 3/5] chore(formatting): Apply flake8 formatting --- parsons/geocode/census_geocoder.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/parsons/geocode/census_geocoder.py b/parsons/geocode/census_geocoder.py index f474893640..b8542b4353 100644 --- a/parsons/geocode/census_geocoder.py +++ b/parsons/geocode/census_geocoder.py @@ -106,8 +106,10 @@ def geocode_address_batch(self, table): logger.info(f"Geocoding {table.num_rows} records.") if set(table.columns) != {"street", "city", "state", "zip"}: - msg = "Table must ONLY include `['id', 'street', 'city', 'state', 'zip']` as" + \ - "columns. Tip: try using `table.cut()`" + msg = ( + "Table must ONLY include `['id', 'street', 'city', 'state', 'zip']` as" + + "columns. Tip: try using `table.cut()`" + ) raise ValueError(msg) chunked_tables = table.chunk(BATCH_SIZE) From 2495e41e1665b25beea8491595e998814c6d6059 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Fri, 7 Jul 2023 18:26:02 -0700 Subject: [PATCH 4/5] feat(an): linting --- parsons/action_builder/action_builder.py | 2 +- parsons/action_network/action_network.py | 11 +++++++++-- test/test_action_network/test_action_network.py | 12 ++++++++---- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/parsons/action_builder/action_builder.py b/parsons/action_builder/action_builder.py index 17aea11fa2..2af620d77f 100644 --- a/parsons/action_builder/action_builder.py +++ b/parsons/action_builder/action_builder.py @@ -279,7 +279,7 @@ def add_section_field_values_to_record( { "action_builder:name": tag, "action_builder:field": field, - "action_builder:section": section + "action_builder:section": section, } for field, tag in field_values.items() ] diff --git a/parsons/action_network/action_network.py b/parsons/action_network/action_network.py index 07b1c9215e..f6ae66170e 100644 --- a/parsons/action_network/action_network.py +++ b/parsons/action_network/action_network.py @@ -1,9 +1,10 @@ import json -from parsons import Table +import logging import re + +from parsons import Table from parsons.utilities import check_env from parsons.utilities.api_connector import APIConnector -import logging logger = logging.getLogger(__name__) @@ -94,6 +95,7 @@ def upsert_person( given_name=None, family_name=None, tags=None, + remove_tags=None, languages_spoken=None, postal_addresses=None, mobile_number=None, @@ -126,6 +128,8 @@ def upsert_person( The person's family name tags: Optional field. A list of strings of pre-existing tags to be applied to the person. + remove_tags: + Optional field. A list of strings of pre-existing tags to be removed from the person languages_spoken: Optional field. A list of strings of the languages spoken by the person postal_addresses: @@ -210,6 +214,9 @@ def upsert_person( data["person"]["postal_addresses"] = postal_addresses if tags is not None: data["add_tags"] = tags + if remove_tags is not None: + data["remove_tags"] = remove_tags + data["person"]["custom_fields"] = {**kwargs} response = self.api.post_request( url=f"{self.api_url}/people", data=json.dumps(data) diff --git a/test/test_action_network/test_action_network.py b/test/test_action_network/test_action_network.py index c6984d3861..f1e5054dee 100644 --- a/test/test_action_network/test_action_network.py +++ b/test/test_action_network/test_action_network.py @@ -1,9 +1,10 @@ -import unittest -import requests_mock import json -from parsons import Table, ActionNetwork +import unittest from test.utils import assert_matching_tables +import requests_mock +from parsons import ActionNetwork, Table + class TestActionNetwork(unittest.TestCase): @requests_mock.Mocker() @@ -435,7 +436,10 @@ def test_update_person(self, m): ) self.assertEqual( self.an.update_person( - self.fake_person_id_1, given_name="Flake", family_name="McFlakerson" + self.fake_person_id_1, + given_name="Flake", + family_name="McFlakerson", + remove_tags=["Fake Tag"], ), self.updated_fake_person, ) From 11d7713f12488092fa0b8f0fb7896bb9232e1f68 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Thu, 20 Jul 2023 19:05:47 -0700 Subject: [PATCH 5/5] chore: lint tests --- .../test_action_builder.py | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/test/test_action_builder/test_action_builder.py b/test/test_action_builder/test_action_builder.py index 4e8e00676e..79b1ae89bf 100644 --- a/test/test_action_builder/test_action_builder.py +++ b/test/test_action_builder/test_action_builder.py @@ -127,7 +127,7 @@ def setUp(self, m): self.fake_field_values = { "Fake Field 2": "Fake Tag 5", - self.fake_field_1: self.fake_tag_4 + self.fake_field_1: self.fake_tag_4, } self.fake_tagging = [ @@ -140,7 +140,7 @@ def setUp(self, m): "action_builder:name": "Fake Tag 5", "action_builder:field": "Fake Field 2", "action_builder:section": self.fake_section, - } + }, ] self.fake_entity_id = "fake-entity-id-1" @@ -162,7 +162,7 @@ def setUp(self, m): "status": "unsubscribed", "source": "api", } - ] + ], } self.fake_upsert_person = { @@ -179,9 +179,9 @@ def setUp(self, m): "action_builder:identifier": "action_builder:fake-email-id-1", "address": "fakey@mcfakerson.com", "address_type": "Work", - "status": "unsubscribed" + "status": "unsubscribed", } - ] + ], } } @@ -197,7 +197,7 @@ def setUp(self, m): "created_date": self.fake_datetime, "modified_date": self.fake_datetime, } - } + }, } self.fake_update_person = { @@ -234,7 +234,8 @@ def test_get_all_records(self, m): text=json.dumps({"_embedded": {"osdi:tags": []}}), ) assert_matching_tables( - self.bldr._get_all_records(self.campaign, "tags"), Table(self.fake_tags_list) + self.bldr._get_all_records(self.campaign, "tags"), + Table(self.fake_tags_list), ) @requests_mock.Mocker() @@ -274,14 +275,15 @@ def prepare_dict_key_intersection(self, dict1, dict2): # Internal method to compare a reference dict to a new incoming one, keeping only common # keys whose values are not lists (i.e. nested). - common_keys = {key for key, value in dict1.items() if key in dict2 - and not isinstance(value, list)} + common_keys = { + key + for key, value in dict1.items() + if key in dict2 and not isinstance(value, list) + } - dict1_comp = {key: value for key, value in dict1.items() - if key in common_keys} + dict1_comp = {key: value for key, value in dict1.items() if key in common_keys} - dict2_comp = {key: value for key, value in dict2.items() - if key in common_keys} + dict2_comp = {key: value for key, value in dict2.items() if key in common_keys} return dict1_comp, dict2_comp @@ -291,7 +293,9 @@ def test_upsert_entity(self, m): # Flatten and remove items added for spreadable arguments upsert_person = self.fake_upsert_person["person"] - upsert_response = self.bldr._upsert_entity(self.fake_upsert_person, self.campaign) + upsert_response = self.bldr._upsert_entity( + self.fake_upsert_person, self.campaign + ) person_comp, upsert_response_comp = self.prepare_dict_key_intersection( upsert_person, upsert_response @@ -348,15 +352,13 @@ def tagging_callback(self, request, context): tagging_data = post_data["add_tags"] # Force the sort to allow for predictable comparison - return sorted(tagging_data, key=lambda k: k['action_builder:name']) + return sorted(tagging_data, key=lambda k: k["action_builder:name"]) @requests_mock.Mocker() def test_add_section_field_values_to_record(self, m): m.post(f"{self.api_url}/people", json=self.tagging_callback) add_tags_response = self.bldr.add_section_field_values_to_record( - self.fake_entity_id, - self.fake_section, - self.fake_field_values + self.fake_entity_id, self.fake_section, self.fake_field_values ) self.assertEqual(add_tags_response, self.fake_tagging)