Feature/point of interest structure upgrade #22

moorchegue · 2022-06-07T04:41:28Z

Addresses #21.

This is really a work in progress, but I'd like to share it early anyway, since it might help catch some gotchas early on as well.

littlepea · 2022-06-07T04:52:21Z

adapters/spreadsheet_adapter.py

@@ -19,12 +19,16 @@ def transform(self, source: List[SpreadsheetRow]) -> List[PointOfInterest]:
        return [self.transform_row(row) for row in source]

    def transform_row(self, row: SpreadsheetRow) -> PointOfInterest:
+        rename = {


Why not use existing ALIASES above?

You're right, exactly same purpose.

littlepea · 2022-06-07T04:53:42Z

config/constants.py

@@ -1,5 +1,8 @@
 from typing import Set

+
+DEFAULT_CATEGORY = 'Any help'


This has already been added by @idisblueflash so your PR should not have this change, please, merge/rebase the latest master

Yeah, bad double rebase…

littlepea · 2022-06-07T04:54:41Z

models/point_of_interest.py

+    country: str = ''
+    city: str = ''
+    address: str = ''
+    categories: List[str] = []


Make sure that these convert correctly into CSV and don't break existing unit tests for spiders

The tests I did fix, but export to CSV might actually work a bit differently. Let me poke around it a bit more…

littlepea · 2022-06-07T04:55:58Z

scraping/spiders/canada_immigration_services.py

+
+log = logging.getLogger(__name__)
+
+


Why is this file here? Are you combining 2 PRs in one?

I actually thought that it was merged. Is it okay to merge it?

Yes, you can merge it

Cool, thanks!

littlepea · 2022-06-07T04:57:31Z

tests/test_convert_data.py

@@ -79,8 +79,15 @@ def test_spreadsheet_data_conversion():

    result = usecase.convert_spreadsheet("some-id")
    csv = FILE_VALUES[0]
-    expected_row = "The Ukrainian House,Poland,Warsaw,\"ul. Zamenhofa 1, 00-153\",52.24734033,20.9964833," \
-                   "Accommodation,Fundacja “Nasz Wybór”,Crisis support center"
+    expected_row = 'The Ukrainian House,Poland,Warsaw,' \


Are you using some auto-formatter that converts all quotes to single-quotes?

Yeah, one style of quotes does make it a bit easier for me. Do we have a preference what quotes to use?

I agree that one style is better, I don't have a preference but I was leaning towards double-quotes because is later we'll enable some auto-formatter like black it's going to force double-quotes

No problem, I'll go with that from now on then.

moorchegue · 2022-06-07T06:31:49Z

As I was fixing moldova_dopomoga crawler (probably source data changed), I noticed that there's also a map there (now): https://www.google.com/maps/d/embed?mid=1RiyRvxOE8q3Z-eo7e298nLn_I0nKXqq6&ehbc=2E312F&ll=47.152161219376964%2C28.378650750000023&z=8

It seems to have all the same info, and also coordinates for all the points. Would you like me to update the crawler to scrape that info?

littlepea · 2022-06-07T06:39:50Z

As I was fixing moldova_dopomoga crawler (probably source data changed), I noticed that there's also a map there (now): https://www.google.com/maps/d/embed?mid=1RiyRvxOE8q3Z-eo7e298nLn_I0nKXqq6&ehbc=2E312F&ll=47.152161219376964%2C28.378650750000023&z=8

It seems to have all the same info, and also coordinates for all the points. Would you like me to update the crawler to scrape that info?

It would be nice if it's easy to do 👍

moorchegue · 2022-06-07T06:46:33Z

Sure, no problem!

wdunn001 · 2022-06-09T19:56:28Z

Make sure to pull latest.

moorchegue · 2022-06-10T11:47:00Z

Hm… looks like poetry install fails on some package in the CI… Have anybody ran into this? I installed locally into a venv with pip install -e . , that worked.

Other than that I think I fixed all outstanding issues. Could you guys take a peek, please?

littlepea · 2022-06-10T13:59:05Z

Hm… looks like poetry install fails on some package in the CI… Have anybody ran into this? I installed locally into a venv with pip install -e . , that worked.

Other than that I think I fixed all outstanding issues. Could you guys take a peek, please?

On master branch CI is passing...

@wdunn001 any ideas?

littlepea · 2022-06-10T14:00:35Z

models/point_of_interest.py

+    email: str = ''
+    url: str = ''
+    socialmedia: str = ''
+    messenger: str = ''


While you were working on it they've add two more fields to the schema 😂

messenger -> FB messenger telegram -> telegram whatsapp -> whatsapp

wdunn001 · 2022-06-10T15:36:23Z

appears to be related to this python-poetry/poetry#728.

Seems there is an outage in PyPi judging from their status page. Probably not related to poetry.

https://status.python.org/

https://stackoverflow.com/questions/72551057/poetry-gives-toomanyindirects-error-suddenly

wdunn001 · 2022-06-10T17:05:32Z

checked tests are working correctly now something in your latest PR is breaking tests.

littlepea · 2022-06-11T00:42:23Z

models/point_of_interest.py

+    email: str = ''
+    url: str = ''
+    socialmedia: str = ''
+    fb_messenger: str = ''


The name of the fields should still be just messenger, "FB messenger" was the meaning

littlepea · 2022-06-11T00:45:00Z

scraping/spiders/moldova_dopomoga.py

+
+
+# XXX: there must be some that encodes/decodes this kind of structure
+def parse_points(json_xml):


Maybe add a dedicated unit test for this function so that we can see what kind of JSON you're parsing?

In theory, you could deserialize it into a pydantic model

littlepea · 2022-06-11T03:00:02Z

I'm gonna merge it so that we can have the new schema on master, we can address any remaining issues in a separate PR

moorchegue force-pushed the feature/point_of_interest_structure_upgrade branch from 721f8db to a93833e Compare June 7, 2022 04:49

littlepea reviewed Jun 7, 2022

View reviewed changes

moorchegue force-pushed the feature/point_of_interest_structure_upgrade branch from dc4324a to d9e2b61 Compare June 7, 2022 06:50

murchik added 5 commits June 10, 2022 19:36

Upgrade PointOfInterest structure.

034971b

Upgrade PointOfInterest structure, more changes after fresh rebase.

d2cd13e

Fix moldova_dopomoga crawler.

2d0304b

moldova_dopomoga new source.

3296c96

A couple of fixes.

19fc0f0

moorchegue force-pushed the feature/point_of_interest_structure_upgrade branch from 028e2c8 to 19fc0f0 Compare June 10, 2022 11:37

littlepea reviewed Jun 10, 2022

View reviewed changes

More messenger fields.

b36f777

Tres commas fix.

55f5f43

littlepea reviewed Jun 11, 2022

View reviewed changes

littlepea merged commit af0345e into safe-refuge:master Jun 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/point of interest structure upgrade #22

Feature/point of interest structure upgrade #22

moorchegue commented Jun 7, 2022

littlepea Jun 7, 2022

moorchegue Jun 7, 2022

littlepea Jun 7, 2022

moorchegue Jun 7, 2022

littlepea Jun 7, 2022

moorchegue Jun 7, 2022

littlepea Jun 7, 2022

moorchegue Jun 7, 2022

littlepea Jun 7, 2022

moorchegue Jun 7, 2022

littlepea Jun 7, 2022

moorchegue Jun 7, 2022

littlepea Jun 7, 2022

moorchegue Jun 7, 2022

moorchegue commented Jun 7, 2022

littlepea commented Jun 7, 2022

moorchegue commented Jun 7, 2022

wdunn001 commented Jun 9, 2022

moorchegue commented Jun 10, 2022

littlepea commented Jun 10, 2022

littlepea Jun 10, 2022

wdunn001 commented Jun 10, 2022 •

edited

Loading

wdunn001 commented Jun 10, 2022 •

edited

Loading

littlepea Jun 11, 2022

littlepea Jun 11, 2022

littlepea commented Jun 11, 2022

		@@ -1,5 +1,8 @@
		from typing import Set


		DEFAULT_CATEGORY = 'Any help'



		# XXX: there must be some that encodes/decodes this kind of structure
		def parse_points(json_xml):


		log = logging.getLogger(__name__)

Feature/point of interest structure upgrade #22

Feature/point of interest structure upgrade #22

Conversation

moorchegue commented Jun 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moorchegue commented Jun 7, 2022

littlepea commented Jun 7, 2022

moorchegue commented Jun 7, 2022

wdunn001 commented Jun 9, 2022

moorchegue commented Jun 10, 2022

littlepea commented Jun 10, 2022

Choose a reason for hiding this comment

wdunn001 commented Jun 10, 2022 • edited Loading

wdunn001 commented Jun 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littlepea commented Jun 11, 2022

wdunn001 commented Jun 10, 2022 •

edited

Loading

wdunn001 commented Jun 10, 2022 •

edited

Loading