Skip to content

Added Place Nodes To Database#435

Merged
maxaaronson merged 9 commits intocodeforboston:mainfrom
maxaaronson:main
May 14, 2025
Merged

Added Place Nodes To Database#435
maxaaronson merged 9 commits intocodeforboston:mainfrom
maxaaronson:main

Conversation

@maxaaronson
Copy link
Collaborator

Added Place, City, State, County classes

@mikeyavorsky mikeyavorsky requested a review from DMalone87 April 1, 2025 23:36
return f"<Place {self.name}>"


class State(Place):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

State, City, etc. don't need to be subclasses of Place. The Nodes will have both the Place AND City,State or other label. Making them subclasses of Place would be redundant.

Copy link
Collaborator Author

@maxaaronson maxaaronson Apr 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the way to set multiple labels on a node with __label__ = 'Label1:Label2' ?
It sounds like this isn't recommended because it can cause issues with lookups:
https://stackoverflow.com/questions/29561962/using-multiple-labels-with-neomodel

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxaaronson maxaaronson reopened this Apr 15, 2025
@DMalone87
Copy link
Collaborator

@maxaaronson - I've started working on implementing this for the data loader. I made a few changes around the relationships.
National-Police-Data-Coalition/data-loaders#7

@maxaaronson maxaaronson marked this pull request as ready for review May 10, 2025 23:49
Copy link
Collaborator

@DMalone87 DMalone87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small changes are we're good to go.

# via
# mixpanel
# python-dateutil
shapely==2.0.7
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add this requirement to requirements/_core.in instead so it makes it into every build.



class CityNode(Place):
population = StringProperty()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this before, but this should be an IntegerProperty. I'll fix this in the loader as well.

@maxaaronson
Copy link
Collaborator Author

Was I supposed to take the shapely req out of dev_unix or have it in both places?

@DMalone87
Copy link
Collaborator

It's cleaner if you remove it, but I don't think it'll break anything.

@maxaaronson maxaaronson merged commit d9d0de9 into codeforboston:main May 14, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants