pysal / tobler

Spatial interpolation, Dasymetric Mapping, & Change of Support
https://pysal.org/tobler
BSD 3-Clause "New" or "Revised" License
145 stars 30 forks source link

Problem running area_interpolate with both intensive and extensive vars #68

Closed pattyf closed 4 years ago

pattyf commented 4 years ago

When I use the the area_interpolate function and include both intensive and extensive variables the output is not what I expected - the rows for each feature in the output geodataframe are repeated. I think this is because the following in line in the function should have the axis set to 1 and not to zero.

https://github.com/pysal/tobler/blob/b3808eb3b532c6a4fd9baacdcc453c7021b2b1ab/tobler/area_weighted/area_weighted.py#L418

Also, I think the target geometry of the area_interpolate function needs to have rows zero indexed. If they are not then this next line does not correctly associate the geometry in the output geodataframe:
https://github.com/pysal/tobler/blob/b3808eb3b532c6a4fd9baacdcc453c7021b2b1ab/tobler/area_weighted/area_weighted.py#L419

Would it make sense to change that to the following? df["geometry"] = target_df["geometry"].reset_index(drop=True)

I'm attaching my notebook and data in a zip file where I tested the above to make what I am trying to do clearer. Thanks for the great library.

tobler_ai_test.zip

knaaptime commented 4 years ago

hey, thanks for the report! I think you're right--looks like we're concatenating along the wrong index (I also notice we're not testing for this, so that should be addressed as well #69 )

since we're adding a column blindly (rather than doing a .join) i'm not sure the change to l419 is technically necessary? I could be mistaken, but another alternative might be df["geometry"] = target_df["geometry"].values which is effectively the same thing (save that you're getting a numpy array versus a pandas series), but either way we should be ignoring the series index? Regardless, i think the change you propose might help make it a bit more explicit, so we'd accept a PR if you wanted to put one together :)

If not, i can get to this in the next day or two

pattyf commented 4 years ago

Hi, I submitted a pull request. Thanks for the feedback.

sjsrey commented 4 years ago

The fix also has to be applied on line 297.