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

Functional blocks #580

Merged
merged 7 commits into from
Jun 4, 2024
Merged

Functional blocks #580

merged 7 commits into from
Jun 4, 2024

Conversation

u3ks
Copy link
Collaborator

@u3ks u3ks commented May 16, 2024

functional implementation of blocks, the code is pretty much the same as the OO version

Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.9%. Comparing base (4037c70) to head (cd275fe).
Report is 26 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #580     +/-   ##
=======================================
+ Coverage   97.4%   97.9%   +0.5%     
=======================================
  Files         26      37     +11     
  Lines       4328    5491   +1163     
=======================================
+ Hits        4214    5375   +1161     
- Misses       114     116      +2     
Files Coverage Δ
momepy/functional/_elements.py 100.0% <100.0%> (ø)
momepy/functional/tests/test_elements.py 100.0% <100.0%> (ø)

... and 3 files with indirect coverage changes

momepy/functional/_elements.py Outdated Show resolved Hide resolved
tessellation: GeoDataFrame,
edges: GeoDataFrame,
buildings: GeoDataFrame,
id_name: str = "bID",
Copy link
Member

Choose a reason for hiding this comment

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

no need for that, rely on index

edges: GeoDataFrame,
buildings: GeoDataFrame,
id_name: str = "bID",
):
Copy link
Member

Choose a reason for hiding this comment

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

missing output type

@u3ks u3ks requested a review from martinfleis June 4, 2024 13:36
Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

We should also make a note that this assumes morphological tessellation and 1:1 relationship between buildings and cells. With enclosed tessellation, those cells that do not have buildings would cause a trouble here. No need to change anything, just adding a note to the docstring would do.


# assign block ids to buildings and tessellations
centroids_w_bl_id2 = gpd.sjoin(buildings_c, blocks, how="left", predicate="within")
# return blocks, centroids_w_bl_id2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# return blocks, centroids_w_bl_id2

@martinfleis martinfleis merged commit c8f9b9b into pysal:main Jun 4, 2024
14 checks passed
@martinfleis martinfleis added enhancement New feature or request and removed refactor labels Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants