uscuni / simplification

Simplification of street network geometry
Other
6 stars 0 forks source link

H3 remake anvy #48

Closed anastassiavybornova closed 1 month ago

anastassiavybornova commented 1 month ago

adding a remake of the workflow to

happy to hear your thoughts on this! also, i'm pretty sure the cellwise metrics computation can be made much more efficient than it is now (cf the notebook cell with

# geom counts in cell
grid["geom_count_base"] = grid.apply(lambda x: geom_count(base, x.geometry), axis = 1)

etc., happy to hear suggestions

anastassiavybornova commented 1 month ago

hi! @martinfleis and @jGaboardi i've implemented all of your changes suggested above, plus the clipping only once-per-apply func as discussed yesterday. from my side it's ready to merge, and @jGaboardi you're obviously (as hinted above) always welcome to do your james stuff on it.

jGaboardi commented 1 month ago

My approval still stands, but we'll wait to see if @martinfleis has any more comments.

jGaboardi commented 1 month ago

I think that you are not using the pre-commit hook @jGaboardi set but apart from that, all good.

@anastassiavybornova Can you run ruff manually on the code/ directory to see if it complains?

ruff check code
anastassiavybornova commented 1 month ago

hmmm that's weird, i am using the pre commit hooks, and i did run the manual ruff check on the utils file before,

(simplification) anvy@mac622265  simplification % ruff check code/utils.py      
All checks passed!
anastassiavybornova commented 1 month ago

ruff check code/ does throw a bunch of errors for four of the notebooks (evaluate_h3cells, momepy, parenx, and _add_artifacts), but I think that's not what you were referring to @martinfleis ?

martinfleis commented 1 month ago

Somehow pre-commit is passing but ruff check complains. Not sure why. I fixed some bits, so let's merge this. We can look into the pre-commit issue later.