CityScope / CS_Urban_Indicators

Scripts for getting urban data and building urban indicators. A module for computing urban indicators in response to real-time CityScope inputs.
GNU General Public License v3.0
7 stars 4 forks source link

Pass geogrid_props to Indicators #16

Closed crisjf closed 4 years ago

crisjf commented 4 years ago

[Issue mainly for documentation]

When defined, tables can have static GEOGRID/properties needed for calculating the indicators.

We need to make sure these properties are passed to the indicators by the Handler and that they are retrievable from the indicator for developing.

image

crisjf commented 4 years ago

For example, to develop an Economic indicator, someone might start from the base class for economic indicators and then link it to the table without instantiating the Handler:

base = EconomicIndicatorBase()
base.link_table('corktown')

Among other things, the base economic indicator knows how to parse NAICS information from the GEOGRID data;

geogrid_data = base.get_geogrid_data()
industry_composition  = base.grid_to_industries(geogrid_data)

When linked, it returns:

{'4451': 75.83333333333333,
 '4453': 75.83333333333333,
 '7224': 301.6666666666666,
 '7225': 605.0,
 '4841': 1149.9999999999993,
 '5617': 2299.9999999999986,
 '5619': 2299.9999999999986,
 '3336': 562.5,
 '5414': 1275.0,
 '5415': 637.5,
 '5417': 1275.0,
 '7211': 575.0}
crisjf commented 4 years ago

@doorleyr let me know what you think of this. It is an extension of the solution you committed in 4daebe9b48d893f6972c1862bc43ba21b11506f8 but it should make developing the indicators easier

crisjf commented 4 years ago

With the last commit, the syntax becomes:

I = InnoIndicator(table_name='corktown')
geogrid_data = I.get_geogrid_data()
I.return_indicator(geogrid_data)
doorleyr commented 4 years ago

One issue with this last commit is that for some of the indicators I need to always pass in the table_name (eg. because the model is trained specifically for each context so I need to load the right model). Up to now i was passing in a table_name on initialisation and retrieving it from the kwargs. With this latest commit, I would not be able to pass a table_name when deploying, right? What if we have table_name as an optional parameter but we don't link the table by default. The syntax would be ''' I = InnoIndicator(table_name='corktown') I.link_table() ''' Of course, if the indicator does not have a table_name from initialisation, then link_table() would cause an error.

crisjf commented 4 years ago

Should work now. Let me know if you have any issues though.

doorleyr commented 4 years ago

I think it's better if we don't call link_table automatically on init() because it means I'm unable to create an indicator with table_name (needed to load the right trained model) without creating a handler and accessing cityIO. I sometimes work with the indicators offline without any handler. This is especially important when cityIO is down (like today). I think we should just remove this part from the init() function and then whenever someone wants to link an indicator to a table (without adding it to a handler), they can just add one extra line of code: indicator.link_table(table_name). If it's ok with you, I can commit the change?

crisjf commented 4 years ago

This makes sense! I just committed the rollback.

I = InnoIndicator()
I.link_table(table_name='corktown')
geogrid_data = I.get_geogrid_data()
I.return_indicator(geogrid_data)