Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to Mapit::Wrapper and Mapit::Place #87

Merged
merged 6 commits into from
Mar 3, 2017
Merged

Improvements to Mapit::Wrapper and Mapit::Place #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bed4b3a
Rename Mapit::Place's 'place' instance variable to 'mapit_area_data'
mhl Mar 1, 2017
b57da5c
Remove hardcoded area types from Mapit::Wrapper
mhl Mar 1, 2017
35a7e2d
Simplify Mapit::Wrapper and make it more efficient
mhl Mar 1, 2017
01c4c92
Add current nigeria.mapit.mysociety.org data for FED, SEN and STA
mhl Mar 2, 2017
a53b196
Use the full MapIt data in tests rather than the reduced fixtures
mhl Mar 2, 2017
fdeb31c
Switch to using the local copy of the MapIt Area data
mhl Mar 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions app.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
set :content_dir, File.join(__dir__, 'prose')
set :datasource, ENV.fetch('DATASOURCE', 'https://github.com/everypolitician/everypolitician-data/raw/master/countries.json')
set :index, EveryPolitician::Index.new(index_url: settings.datasource)
set :mapit_url, 'http://nigeria.mapit.mysociety.org/areas/'
set :twitter_user, 'NGShineyoureye'

# Create a wrapper for the mappings between the various IDs we have
Expand All @@ -42,9 +41,10 @@

# Create a wrapper that caches MapIt and EveryPolitician area data:
mapit = Mapit::Wrapper.new(
mapit_url: settings.mapit_url,
mapit_mappings: mapit_mappings,
baseurl: '/place/'
baseurl: '/place/',
area_types: %w(FED SEN STA),
data_directory: 'mapit'
)

# Assemble data on the members of the various legislatures we support:
Expand Down Expand Up @@ -103,17 +103,17 @@
end

get '/place/is/state/' do
@page = Page::Places.new(title: 'States', places: mapit.states, people_by_legislature: governors)
@page = Page::Places.new(title: 'States', places: mapit.places_of_type('STA'), people_by_legislature: governors)
erb :places
end

get '/place/is/federal-constituency/' do
@page = Page::Places.new(title: 'Federal Constituencies (Current)', places: mapit.federal_constituencies, people_by_legislature: representatives)
@page = Page::Places.new(title: 'Federal Constituencies (Current)', places: mapit.places_of_type('FED'), people_by_legislature: representatives)
erb :places
end

get '/place/is/senatorial-district/' do
@page = Page::Places.new(title: 'Senatorial Districts (Current)', places: mapit.senatorial_districts, people_by_legislature: senators)
@page = Page::Places.new(title: 'Senatorial Districts (Current)', places: mapit.places_of_type('SEN'), people_by_legislature: senators)
erb :places
end

Expand Down
39 changes: 12 additions & 27 deletions lib/mapit/place.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,50 +1,35 @@
# frozen_string_literal: true
module Mapit
class Place
def initialize(place:, mapit_ids_to_pombola_slugs:, baseurl:)
@place = place
@mapit_ids_to_pombola_slugs = mapit_ids_to_pombola_slugs
attr_accessor :parent

def initialize(mapit_area_data:, pombola_slug:, baseurl:, parent: nil)
@mapit_area_data = mapit_area_data
@pombola_slug = pombola_slug
@baseurl = baseurl
@parent = parent
end

def id
place['id']
mapit_area_data['id']
end

def name
place['name']
mapit_area_data['name']
end

def type_name
place['type_name']
mapit_area_data['type_name']
end

def parent_name
place['parent_name']
end
alias_method :is_child_area?, :parent

def url
build_url(place['id']) if place['id']
end

def parent_url
build_url(place['parent_id']) if place['parent_id']
end

def is_child_area?
!place['parent_name'].nil?
"#{baseurl}#{pombola_slug}/"
end

private

attr_reader :place, :mapit_ids_to_pombola_slugs, :baseurl

def build_url(id)
"#{baseurl}#{pombola_slug(id)}/"
end

def pombola_slug(id)
mapit_ids_to_pombola_slugs[id.to_s]
end
attr_reader :mapit_area_data, :pombola_slug, :baseurl
end
end
97 changes: 42 additions & 55 deletions lib/mapit/wrapper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,87 +3,74 @@

module Mapit
class Wrapper
def initialize(mapit_url:, mapit_mappings:, baseurl:)
@mapit_url = mapit_url
@mapit_mappings = mapit_mappings
@baseurl = baseurl
end

def all_areas
states + federal_constituencies + senatorial_districts
end

def states
@states ||= areas('STA').map { |area| create_place(area) }
end

def federal_constituencies
@constituencies ||= add_parent_data(areas('FED')).map { |area| create_place(area) }
end

def senatorial_districts
@districts ||= add_parent_data(areas('SEN')).map { |area| create_place(area) }
def initialize(mapit_mappings:, baseurl:, area_types:, data_directory:)
@baseurl = baseurl
@area_types = area_types
@mapit_mappings = mapit_mappings
@data_directory = data_directory
cache_mapit_data
set_up_parent_child_relationships
end

def area_from_pombola_slug(slug)
find_single(pombola_slugs_to_mapit_ids[slug].to_i)
mapit_id = mapit_mappings.pombola_slugs_to_mapit_ids[slug]
id_to_place[mapit_id]
end

def area_from_ep_id(id)
find_single(ep_to_mapit_ids[id].to_i)
id_to_place[mapit_mappings.ep_to_mapit_ids[id]]
end

private

attr_reader :mapit_url, :mapit_mappings, :baseurl

def areas(area_type)
uri = URI(mapit_url + area_type)
JSON.parse(Net::HTTP.get(uri)).values
def places_of_type(area_type)
type_to_places[area_type]
end

def add_parent_data(child_areas)
child_areas.map do |area|
parent = {
'parent_id' => parent_id(area),
'parent_name' => parent_name(area)
}
area.merge(parent)
end
end
private

def parent_id(area)
area['parent_area'] || child_to_parent[area['id'].to_s].to_i
end
attr_reader :mapit_mappings,
:baseurl,
:id_to_place,
:area_types,
:type_to_places,
:data_directory

def parent_name(area)
states.find { |state| state.id == parent_id(area) }.name
def places(area_type)
areas_data(area_type).map { |a| create_place(a) }
end

def find_single(id)
all_areas.find { |area| area.id == id }
def mapit_area_cache_filename(area_type)
File.join(data_directory, "#{area_type}.json")
end

def pombola_slugs_to_mapit_ids
mapit_mappings.pombola_slugs_to_mapit_ids
def parse_json_file(filename)
JSON.parse(open(filename, &:read))
end

def mapit_ids_to_pombola_slugs
mapit_mappings.mapit_ids_to_pombola_slugs
def areas_data(area_type)
parse_json_file(mapit_area_cache_filename(area_type)).values
end

def child_to_parent
mapit_mappings.child_to_parent
def cache_mapit_data
@type_to_places = area_types.map { |t| [t, places(t)] }.to_h
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are building these hashes, it may be a better idea to use symbols for the hash key. This is a common practice in Ruby as symbols are more performant than strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I know that's often done, but I don't think that's worth it in this case. Area type codes often come from MapIt calls, which will return them from JSON strings, and when you use them in queries to MapIt, you need them to be strings then as well. Transforming those strings to and from symbols for any input and output like that seems unnecessary, given that the performance impact from using strings instead of hash keys in going to be tiny in this app.

@id_to_place = type_to_places.values.flatten.map { |a| [a.id.to_s, a] }.to_h
end

def ep_to_mapit_ids
mapit_mappings.ep_to_mapit_ids
def set_up_parent_child_relationships
# FIXME: this should also use MapIt's parent_area, if that's set.
mapit_mappings.child_to_parent.each do |child, parent|
if id_to_place.key?(child) && id_to_place.key?(parent)
id_to_place[child].parent = id_to_place[parent]
end
end
end

def create_place(area)
def create_place(mapit_area_data)
mapit_id = mapit_area_data['id'].to_s
pombola_slug = mapit_mappings.mapit_ids_to_pombola_slugs[mapit_id]
Mapit::Place.new(
place: area,
mapit_ids_to_pombola_slugs: mapit_ids_to_pombola_slugs,
mapit_area_data: mapit_area_data,
pombola_slug: pombola_slug,
baseurl: baseurl
)
end
Expand Down
1 change: 1 addition & 0 deletions mapit/FED.json
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions mapit/SEN.json
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions mapit/STA.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"2": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 2, "codes": {"poll_unit": "AB"}, "name": "Abia", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "3": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 3, "codes": {"poll_unit": "AD"}, "name": "Adamawa", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "4": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 4, "codes": {"poll_unit": "AK"}, "name": "Akwa Ibom", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "5": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 5, "codes": {"poll_unit": "AN"}, "name": "Anambra", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "6": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 6, "codes": {"poll_unit": "BA"}, "name": "Bauchi", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "7": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 7, "codes": {"poll_unit": "BY"}, "name": "Bayelsa", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "8": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 8, "codes": {"poll_unit": "BE"}, "name": "Benue", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "9": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 9, "codes": {"poll_unit": "BO"}, "name": "Borno", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "10": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 10, "codes": {"poll_unit": "CR"}, "name": "Cross River", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "11": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 11, "codes": {"poll_unit": "DE"}, "name": "Delta", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "12": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 12, "codes": {"poll_unit": "EB"}, "name": "Ebonyi", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "13": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 13, "codes": {"poll_unit": "ED"}, "name": "Edo", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "14": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 14, "codes": {"poll_unit": "EK"}, "name": "Ekiti", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "15": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 15, "codes": {"poll_unit": "EN"}, "name": "Enugu", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "16": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 16, "codes": {"poll_unit": "FC"}, "name": "Federal Capital Territory", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "17": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 17, "codes": {"poll_unit": "GO"}, "name": "Gombe", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "18": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 18, "codes": {"poll_unit": "IM"}, "name": "Imo", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "19": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 19, "codes": {"poll_unit": "JI"}, "name": "Jigawa", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "20": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 20, "codes": {"poll_unit": "KD"}, "name": "Kaduna", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "21": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 21, "codes": {"poll_unit": "KN"}, "name": "Kano", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "22": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 22, "codes": {"poll_unit": "KT"}, "name": "Katsina", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "23": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 23, "codes": {"poll_unit": "KE"}, "name": "Kebbi", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "24": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 24, "codes": {"poll_unit": "KO"}, "name": "Kogi", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "25": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 25, "codes": {"poll_unit": "KW"}, "name": "Kwara", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "26": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 26, "codes": {"poll_unit": "LA"}, "name": "Lagos", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "27": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 27, "codes": {"poll_unit": "NA"}, "name": "Nassarawa", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "28": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 28, "codes": {"poll_unit": "NI"}, "name": "Niger", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "29": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 29, "codes": {"poll_unit": "OG"}, "name": "Ogun", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "30": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 30, "codes": {"poll_unit": "ON"}, "name": "Ondo", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "31": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 31, "codes": {"poll_unit": "OS"}, "name": "Osun", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "32": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 32, "codes": {"poll_unit": "OY"}, "name": "Oyo", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "33": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 33, "codes": {"poll_unit": "PL"}, "name": "Plateau", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "34": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 34, "codes": {"poll_unit": "RI"}, "name": "Rivers", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "35": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 35, "codes": {"poll_unit": "SO"}, "name": "Sokoto", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "36": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 36, "codes": {"poll_unit": "TA"}, "name": "Taraba", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "37": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 37, "codes": {"poll_unit": "YO"}, "name": "Yobe", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}, "38": {"parent_area": null, "generation_high": 1, "all_names": {}, "id": 38, "codes": {"poll_unit": "ZA"}, "name": "Zamfara", "country": "N", "type_name": "State", "generation_low": 1, "country_name": "Nigeria", "type": "STA"}}
5 changes: 0 additions & 5 deletions tests/fixtures/mapit_data.rb
View file
Open in desktop

This file was deleted.

Loading