Locations (now flamewar free!)

[David McCann]

Hey folks--

Couldn't resist making a sarcastic mention of our last attempt at discussing Locations, but I think we're all sort of diverging again so I'd like to revive the conversation.  I made sort of a snarky comment to Adam, and he was kind enough to post back with both 1) his reasoning behind locations, 2) a history of previous location designs and 3) a justification for the current approach, which I had clobbered by making Locations concrete and adding Location types again.  I'm appending his comments here.  They're deep within github commit logs, and I think it's really good stuff so I wanted to share.  I of course couldn't resist adding my $0.02 below all that (sorry for the long email).

Just to summarize (Adam, step in if I'm not paraphrasing well):

1) Location models are definitely hard to get right, and each deployment typically has very specific needs

2) There seems to be some debate over the OO principles "is-a" and "has-a": what things actually *are* locations, and should subclass or just use the base, and which things *have* locations, and just need a foreign key?

---From Adam:

i fixed the ui breakage in rapidsms/rapidsms@fb7a1ebf03d87ee6320e6e9bf26869c779369a46 [this was my above-mentioned commit], so forget about that.

i don't see how this solution to locations (a single `Location` model with associated `LocationType`s) would be sufficient for anything but the simplest projects. i removed it ages ago, because we tried it in the field a few times, and it wasn't flexible enough. i'm not saying my solution is perfect (actually, it's incomplete and sucks), but it more accurately models the real world, which is surely the point of _models_.

most deployments require multiple types of location. in ethiopia, for example, there were regions, districts (woredas), and health centers (otps). we had the latitude and longitude for each, but they were otherwise totally different models: regions had a name, and many districs; districts had explicit codes, and many health centers; health centers had auto-generated codes and many reporters; supplies could be delivered to districts or health centers, but not regions; stock outs were only reported at health centers.

this fairly typical situation could not be accurately modeled with a single `Location` model, so we broke it into multiple models, and duplicated some of the functionality. that sucked, because it meant we had to build our own mapping ui (again), which couldn't easily be shared with other deployments, because it was full of project-specific code. next time around (nigeria, i think), we tried to make a single monolithic `Location` model which would solve all of the problems from the previous deployment. that sucked even more, because it was complicated _and_ insufficient.

i won't talk about trying to combine multiple location-aware apps in the same project here, but as you can imagine, that was a cluster-fuck, too. when i tried to create a demo project (for an event) containing some of dimagi's stuff, a couple of unicef deployments, and matt's mctc thing, there were no less than three separate maps (two google, one openlayers), and four create/read/update/delete interfaces for various location types. collaboration fail.

the compromise which we eventually came up with was the `Location`-subclassing approach, which i'm still advocating for. rather than defining our own models in contrib (which always seem to be insufficient), i would prefer to provide a small-ish base class, which projects can subclass and customize however they like. since the functionality of the base class is constant, we can (and have) built a useful ui around it, with the usual CRUD operations, and plotting everything on a single map.

my point is, sure, your approach is simpler for one project, but bad for collaboration between projects. however, i'm not really involved in the project any more, so i'm not trying to prevent anyone from moving forward. just trying to make sure we don't repeat the same mistakes that we've made in the past.

sorry, that was rather long-winded. maybe i should copy it to the mailing list. [done!]

---

My $0.02 is that probably both cases apply to most deployments.  I'd say for instance a health facility *has-a* location, whereas a district certainly *is* one.  For my particular deployments that's the attitude I take: countries, districts, parishes, towns, etc. typically follow *close enough* of the same model that I can get away with having them all in one table, and differentiating by type.  That also makes it quite easy if the needs of the project suddenly need to capture things at the subdistrict or county level, without creating new subclasses/database tables.  Anything else (i.e. water pumps, health facilities) *has-a* location, because certainly they're going to have all sorts of different attributes and business logic.  This doesn't make a generalized mapping app particularly possible, I realize, but then, I tend to think visualizations get pretty deployment-specific as it is.

Only other thing to bear in mind is the in-development (http://docs.rapidsms.org/InDevelopment) simple_locations app, which uses a tree-based library.  Definitely a WIP, but I think if we can cherry-pick what we like about both, come to an agreement about what the base location model should include, and collaborate within common domains where we have to subclass or reference locations (like the health_models app), hopefully we can avoid "collaboration fail."

later,

--dm

[Cory Zue]

Maybe a dumb question but would shipping an abstract Location model
along with a non-abstract stub implementation (e.g. BasicLocation)
with a type suit both philosophies? Or am I missing something
essential in the problem?

I think if we had a few real-world examples of how to extend these
models to suit more complicated use cases on the wiki that might go a
long way towards getting everyone on the same page.

Cory

> --
> You received this message because you are subscribed to the Google Groups
> "rapidsms" group.
> To post to this group, send email to rapi...@googlegroups.com.
> To unsubscribe from this group, send email to
> rapidsms+u...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/rapidsms?hl=en.
>

[Tobias McNulty]

Hey -

So what's the plan for contrib.locations?

I'm honestly pretty indifferent; I just want something that's not going to be changing left and right every couple months.  To that end, I think simplest of solutions--one that doesn't try to hit every use case (it's probably impossible anyways)--is probably the best bet.  But really, I'll use anything that's there and mostly just want something that "basically works."

I ask because I'm dealing with the generic foreign key migration now (why?) and don't want to do it again in the future. :-D

Cheers,

Tobias

--
Tobias McNulty, Managing Partner

Caktus Consulting Group, LLC
http://www.caktusgroup.com

[David McCann]

I think Cory's proposal is definitely the sane approach.  As a community, we can hopefully all agree on:

 - an abstract class, which should be extended for things that you truly feel *are* locations, but need attributes tacked on at the object level

 - a simple concrete class, which can be used for basic hierarchies of locations that are all fairly similar (i.e. just have names and types, like districts, subdistricts, counties, parishes, etc. etc.)

 - *using caution* when extending the abstract class.  I would still argue that water pumps and health facilities *aren't locations*, rather they *have locations* (the whole is-a vs. has-a debate that always crops up in OO).  Otherwise we're going to have all these fragmented subclasses, and lots of migration if we ever want to collaborate more closely

 - using a tree-based library for efficiency

If we can agree to all those, I think we can incorporate simple_locations into contrib.locations; our notion of Area becomes that "simple concrete class" I referred to above, and everything else stays more-or-less the same.  By the way, simple_locations comes with a nice locations management view out-of-the-box for dealing with your concrete instances of location (hopefully a nice  accompaniment to the existing view)?

I only have one question to add though: is a class that's both abstract AND extensible overkill?  I think it again is trying to be so much for so many that it ends up being 1) dangerous for it's openness and 2) not particularly useful to anyone out-of-the-box.  I understand the caution behind designing it this way with no clear requirements, but I hope now that there are enough people out there that can weigh in now so we can decide on something a little more rigid.

--dm

[David McCann]

Er, I let this thread die because I needed to test a hierarchal locations framework in production for a year!

A thin excuse admittedly, and apologies for not being more active on the list lately.  However, it is true that we've been using MPTT for a year now, from the simple_locations repository.

I'm renewing my interest in tweaking the locations models and pulling more nice-to-have functionality into the core, as I know the "vanilla" locations provided in contrib.locations don't work well if you have the following assumptions/requirements:

 - hierarchal locations that represent administrative boundaries are locations, everything else HAS-A location

 - fast aggregation over subtrees in this hierarchy are needed for reporting

I've added rapidsms.contrib.locations.nested to my fork of rapid (there's also another tweak in there for ExtensibleModel, to respect "app_label" Meta attributes, but I can pull that in separately regardless of the community's decision).  It has a dependency on django-mptt, although I don't intend to add this to the setup.py, as it's an *optional* dependency.  Rather, I intend to add a "locations" page to docs.rapidsms.org about how to perform the pip install and update settings.py if you choose to use rapidsms.contrib.locations.nested.

I know that even this will be insufficient for my deployment's needs shortly: for interoperability, we'll need to start integrating with 3rd-party WMS services to synchronize data and perform geospatial queries.  So I sort of look at this as a first stab at tacking on further features to Locations in a way that allows other apps, who may be dependent only upon "vanilla" locations, to still play nice with those apps wanting the additional functionality.  Not to mention I'm sort of over managing about 12 repositories just to get a basic polling app out the door.

Also, I noticed that no one has ever changed the base Locations model back to abstract since I nixed that last year.  Does anyone feel strongly about making it abstract again?  :-D

Happy to merge this extra, very simple app into the main repo if there aren't any objections.

Cheers, and happy (belated) new year!

--dm

[David McCann]

Went ahead an pulled this in since there were no objections.  Looking at my changes from a while ago, I'd probably also like to:

 - move LocationType into contrib.locations.nested, it doesn't really make any sense where it is now

 - optionally make Location abstract (as mentioned above)...again, unless you're using contrib.nested, you really don't want the Location model to be concrete.  Also, any proposals on the best way to do this? :-)  I'm sure I could do it with a little tweaking of ExtensibleModel, is anyone opposed to that?

I also added a page to the wiki summarizing the differences between these two apps:

http://docs.rapidsms.org/Locations

Cheers,

--dm