coderholic / django-cities

Countries and cities of the world for Django projects
MIT License
927 stars 371 forks source link

Implemented "nearest_to" method for Place managers. #105

Closed cguethle closed 8 years ago

cguethle commented 8 years ago

This allows any Place model to ask its manager (assuming it has a location field) if a particular Point (long/lat) is within a certain number of miles of a known Place of that type in the database.

Example. I know I have a long/lat of "-96.8028" and "32.7791". I want to find the closest City within 10 miles of that Point.

my_point = Point(-96.8028, 32.7791)
miles_within = 10
closest_city = City.objects.nearest_to(my_point, miles_within)
""" :type : cities.models.City """
if closest_city:
    print "Closest city is {city}.".format(city=closest_city)
else:
    print "No cities in the database within {miles} miles of {pnt}".format(miles=miles_within, pnt=my_point)

This is implemented on a new PlaceManager which all Places will get by default. This function requires a location field, so if a Place does not have a location field then a log message will inform on the problem. If a location-less Place wants this functionality, and they have a foreign field that DOES have a location, they can implement their own manager with a nearest_to function and expose that relationship themselves. Additional functionality could also attempt to discover a relationship of that sort automatically.

If this is good, I can update the pull request to have a more appropriate version in the setup.py. I change it to 0.4.2.1 to be able to differentiate in my requirements to pull only from my personal repo.

coderholic commented 8 years ago

I'm not sure the nearest_to method is significantly better or simpler than just doing the following, where you're not limited by range, and you can pull the N nearest if you wish rather than just one:

City.objects.distance(point).order_by('distance')[0]
cguethle commented 8 years ago

I suppose that is fair, and to be honest I didn't realize GeoManager had a distance function already (should have used that in my method). However, a "nearest_to" type method feels more semantically correct when referring to places like Cities, Countries, etc. It is obvious that I am getting the nearest Place to the Point, more readable and easier to remember. With your suggestion, you also need to be prepared to catch an IndexError if your Place dataset is empty (hopefully not).

That was my thought anyways.

coderholic commented 8 years ago

We have examples of .distance in the readme: https://github.com/coderholic/django-cities#examples

Totally agree that nearest_to feels more semantically correct, but it's also less flexible than distance, and it adds a small amount of complexity for the project. Everything considered, I don't think we should include the change.

cguethle commented 8 years ago

shrug Well, your rodeo, so I'll just stick to my fork. Cheers.