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

Contiguity weight docs #706

Open
dfolch opened this issue May 7, 2024 · 4 comments
Open

Contiguity weight docs #706

dfolch opened this issue May 7, 2024 · 4 comments
Assignees
Labels

Comments

@dfolch
Copy link
Member

dfolch commented May 7, 2024

An undocumented (and useful) feature of the Rook and Queen contiguity weights constructors is that passing in points results in the construction of voronoi polygons, which are then used to build contiguity weights. The current docs imply that only polygons can be passed. You need to read the source code to understand how points result in the identification of contiguous neighbors. I propose modifying the Rook and Queen docs to say something like.

class Rook(W):
    """
    Construct a weights object based on polygons that
    that share at least one edge. A collection of pysal
    polygons can be passed in; passing pysal points
    results in voronoi polygons being built first.

    Parameters
    ----------
    polygons    : list
                a collection of PySAL polygons to build weights from 
                (a collection of points results in the construction of
                voronoi polygons)
@martinfleis
Copy link
Member

To be fair, I don't think this behaviour should exist in the first place. Contiguity based on Voronoi polygons around points is not a Queen or Rook contiguity of point geometry. There's an explicit libpysal.weights.Voronoi constructor for that purpose. This is also the reason why we did not mirror this behaviour in Graph.build_contiguity.

@knaaptime
Copy link
Member

i dont have a strong opinion about updating the docstring, but in general its a good idea to think of W as deprecated (even if it hasnt been, formally) and start using Graph whenever possible

also, we're not constructing the W from 'PySAL polygons', so the doc would need a bit more love anyway :P

@martinfleis
Copy link
Member

@knaaptime That's correct for the vanilla __init__, no? You just always call the data frame constructor.

@knaaptime
Copy link
Member

probably, i just figured all the bespoke geometry stuff was gone by this point

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

No branches or pull requests

4 participants