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

Consider documenting units for the max_coverage parameter of MCLP/LSCP.from_geodataframe #241

Closed
apulverizer opened this issue Jun 5, 2022 · 1 comment

Comments

@apulverizer
Copy link

Noticed this as part of openjournals/joss-reviews#3330

It's a bit unclear what the distance unit actually is. I assume it's in the units of the coordinate system that the geodataframes are in. From a usability perspective, it'd be nice to at least document that. Even better, would be to support specifying the units and internally converting it.

Something like:

mclp_from_geodataframe = MCLP.from_geodataframe(
    clients_snapped, 
    facilities_snapped, 
    "geometry", 
    "geometry", 
    "demand", 
    100,
    service_radius_units="miles",
    p_facilities=P_FACILITIES,
    distance_metric="euclidean",
)
@jGaboardi
Copy link
Member

@apulverizer This is a nice suggestion, and one we have considered before. However, as with other PySAL submodules, we expect the user to have performed preprocessing and to know the projection/coordinate system. Therefore, the user must specify only distance unit itself. Considering the naming max_coverage was recently changed to service_radius in order to more accurately reflect facility location terminology.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants