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.

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

second step in building a Shapley's decomposition framework (also address many tweaks, typos, etc.) #16

Merged
merged 17 commits into from May 11, 2019

Conversation

@renanxcortes
Copy link
Collaborator

commented Apr 17, 2019

Folks, this is a second step to build a decomposition framework for the segregation module. Since each of the three counterfactual approaches are very different in terms of generation of the counterfactual populations, I chose to slightly change the previous function building a function (_generate_counterfactual) that returns always the counterfactual group population and the counterfactual total population (note that each of these variables can be generated differently in the generation of each approach, so this facilitates). With this, the _decompose_index can have a more generic structure that facilitates each of the approaches. Finally, I chose to build a 'Decompose_Segregation' class with c_s and c_a attribute of the shapley's decomposition.

I put the "auxiliary" function that generates the counterfactuals in the util.py, because I think we can derive all the methods from the 'mother' class I'm proposing.

I tested all functions with the data of our comparative paper and everything is matching. The way we're recovering the counterfactual value is that elegant one with the rank pandas method.

I think the next step is to build appropriate doctests, plotting method for this class (maps and cfds) (taking advantage of the plotting function already built), notebooks, etc.

It also addresses

#8
and
#9

Please, let me know if you're ok with this structure I'm proposing.

ps.: you can ignore the first two commits cause this was to just to check my github syncronization.

@renanxcortes renanxcortes requested review from knaaptime and sjsrey Apr 17, 2019

return geometry in core_data if input data is geopandas (for plotting…
… maps in wrappers such as the decomposition)
@knaaptime
Copy link
Member

left a comment

i think also the typos in the README's graphics (tracks ==> tracts) managed to find their way back in

@renanxcortes renanxcortes changed the title second step in building a Shapley's decomposition framework second step in building a Shapley's decomposition framework (also address many tweaks, typos, etc.) May 3, 2019

@renanxcortes

This comment has been minimized.

Copy link
Collaborator Author

commented May 6, 2019

Hey, folks, could any of you take a look at this PR to see if it is ok the merge it (@sjsrey or @knaaptime )? I wanted to open another PR for a different task (related to centralization indexes) but I accidentally committed in this branch (but didn't push yet), I should have created a specific branch for this more recent task...

BTW, Eli, the typo of tracks -> tracts is already contemplated in this current PR.

@@ -432,7 +444,11 @@ def _isolation(data, group_pop_var, total_pop_var):
X = x.sum()
xPx = np.nansum((x / X) * (x / t))

core_data = data[['group_pop_var', 'total_pop_var']]
if (str(type(data)) != '<class \'geopandas.geodataframe.GeoDataFrame\'>'):

This comment has been minimized.

Copy link
@knaaptime

knaaptime May 6, 2019

Member

this seems like a kinda convoluted way to do the type check, but not a huge deal

This comment has been minimized.

Copy link
@renanxcortes

renanxcortes May 6, 2019

Author Collaborator

Yes, I agree! Is there a shorter way to do that? I tried to use isinstance, but it assumes the way the user import geopandas:

image

This comment has been minimized.

Copy link
@renanxcortes

renanxcortes May 6, 2019

Author Collaborator

However, the current way works for any different way to import geopandas.
BTW, I really like the asserts you use in geosnap for conditions... I think I'll refactor to that style later.

@sjsrey
sjsrey approved these changes May 11, 2019

@renanxcortes renanxcortes merged commit 6b93fd0 into pysal:master May 11, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.