P dispersion

[erinrolson]

Implement p-dispersion model

[codecov[bot]]

Codecov Report

Merging #268 (92a8211) into main (768ea8a) will increase coverage by 0.7%. The diff coverage is 93.1%.

@@           Coverage Diff           @@
##            main    #268     +/-   ##
=======================================
+ Coverage   67.0%   67.8%   +0.7%     
=======================================
  Files         23      24      +1     
  Lines       2659    2717     +58     
  Branches     583     591      +8     
=======================================
+ Hits        1782    1841     +59     
+ Misses       790     783      -7     
- Partials      87      93      +6     

Impacted Files	Coverage Δ
spopt/locate/p_dispersion.py	90.7% <90.7%> (ø)
spopt/locate/base.py	95.0% <100.0%> (+0.6%)	:arrow_up:
spopt/_version.py	33.8% <0.0%> (+1.5%)	:arrow_up:

[jGaboardi]

Addressing #269.

[erinrolson]

Oops, just saw this after fixing the plot, but your code is much cleaner. I'll play with implementing your solution a bit later, thanks!

On Fri, Aug 19, 2022 at 3:33 PM Germano Barcelos @.***> wrote:

@.**** commented on this pull request.

I just have this comment and it looks nice to me. Again, you are doing an awesome job @erinrolson https://github.com/erinrolson.

In notebooks/facloc-disperse-real-world.ipynb https://github.com/pysal/spopt/pull/268#discussion_r950506058:

• " legend_elements.append(_patch)\n",

• "\n",
• " if 'predefined_loc' in facility_points.columns:\n",
• " \n",
• " pre_fac = facility_points[facility_points.predefined_loc == 1]\n",
• " for i in range(len(pre_fac)):\n",
• " pre_fac.iloc[[i]].plot(ax=ax,\n",
• " marker="*",\n",
• " markersize=200 * 3.0,\n",
• " alpha=0.8,\n",
• " zorder=4,\n",
• " edgecolor="red",\n",
• " facecolor=pre_colors[i])\n",
• "\n",
• " other_fac = facility_points[facility_points.predefined_loc == 0]\n",
• " for i in sited_facilities:\n",

Aha! Gotcha! This is a super tricky thing. In this case: fac_sites = [0, 3, 5, 6, 7, 11] sited_facilities = [0, 1, 2, 3, 4, 5]

other_fac contains stores sites, except predefined ones and it has 16 rows (index ranging from 0 to 15), so when you index with sited_facilities you are using the right indexes but in a dataset that doesn't correspond to the facilities chosen. So, it will print more stores than expected 9 in this case instead of 6.

I recommend taking a look at this gist https://gist.github.com/gegen07/32f961686f4b339870a72291ae32c322 I did which makes it simpler and less repetitive. It also solves the issue of printing more facilities than expected.

— Reply to this email directly, view it on GitHub https://github.com/pysal/spopt/pull/268#pullrequestreview-1079279907, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJKDQOXG32IVDTY2TENQJFLVZ7OQDANCNFSM56IWR6IA . You are receiving this because you were mentioned.Message ID: @.***>

[jGaboardi]

congrats on another successful PR @erinrolson! :tada:

[erinrolson]

Thank you!

On Fri, Sep 9, 2022 at 4:00 PM James Gaboardi @.***> wrote:

congrats on another successful PR @erinrolson https://github.com/erinrolson! 🎉

— Reply to this email directly, view it on GitHub https://github.com/pysal/spopt/pull/268#issuecomment-1242406295, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJKDQOWXBVBOEPXCZGEMKDLV5OJOBANCNFSM56IWR6IA . You are receiving this because you were mentioned.Message ID: @.***>