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

DOC: Improve STRtree docstrings #1626

Merged
merged 3 commits into from Dec 6, 2022

Conversation

brendan-ward
Copy link
Collaborator

Resolves #1622

Improves STRtree docstrings in an attempt to make it more clear that the tree is based on 2-dimensional bounding boxes. Also added a note about tree efficiency, but that might be overkill for the docstring and more appropriate for dedicated documentation on STRtree - which we have not yet created.

/cc @adishavit

@brendan-ward
Copy link
Collaborator Author

Test failures are unrelated, this is a doc only change

@jorisvandenbossche
Copy link
Member

Test failures are unrelated, this is a doc only change

Yes, I am trying to fix those in #1625

Copy link
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely written, and good to distinguish between a geometry's "extent" vs "bounding box", among several other details added.

@jorisvandenbossche
Copy link
Member

good to distinguish between a geometry's "extent" vs "bounding box",

Apart from that "bounding box" is a clearer name (and I assume more people will understand it), what do you understand to be the difference between both? (my assumption was that GEOS uses "extent" term for bounding box)

@jorisvandenbossche
Copy link
Member

Actually, in the STRtree API, they actually use "envelope", not extent (and also the property in shapely is envelope), so yet another term .. :)

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarifications! That are nice improvements, just one nitpicky comment

shapely/strtree.py Outdated Show resolved Hide resolved
@adishavit
Copy link
Contributor

adishavit commented Nov 25, 2022 via email

@mwtoews
Copy link
Member

mwtoews commented Nov 25, 2022

GEOS uses "extent" term for bounding box.

Yes, but as mentioned previously it is not widely understood as bounding box. Agree there are many terms for the same thing.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3551050299

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.754%

Totals Coverage Status
Change from base Build 3538932801: 0%
Covered Lines: 2218
Relevant Lines: 2617

💛 - Coveralls

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jorisvandenbossche jorisvandenbossche merged commit 882bf37 into shapely:main Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: Enhance description of STRtree to indicate that geometries are indexed and queried based on extents
5 participants