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

ENH: neighbor_distance using Graph and new API #555

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

martinfleis
Copy link
Member

Only a boring 10x speedup here 😃

15.3 s ± 362 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
1.5 s ± 4.38 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

It currently prints warnings but those shall be solved by geopandas/geopandas#1832.

@martinfleis martinfleis added the enhancement New feature or request label Mar 6, 2024
@martinfleis martinfleis added this to the 0.8.0 milestone Mar 6, 2024
@martinfleis martinfleis requested a review from jGaboardi March 6, 2024 20:02
@martinfleis martinfleis self-assigned this Mar 6, 2024
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.6%. Comparing base (4037c70) to head (27ce3ca).
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #555     +/-   ##
=======================================
+ Coverage   97.4%   97.6%   +0.2%     
=======================================
  Files         26      30      +4     
  Lines       4328    4752    +424     
=======================================
+ Hits        4214    4638    +424     
  Misses       114     114             
Files Coverage Δ
momepy/functional/_distribution.py 100.0% <100.0%> (ø)
momepy/functional/tests/test_distribution.py 100.0% <100.0%> (ø)

@knaaptime
Copy link
Member

i've spent a bunch of time trying to copy the momepy and spopt test workflow files, but i cannot get that lovely codecov report thing to trigger for any of my packages

@martinfleis
Copy link
Member Author

@knaaptime I believe it is this https://github.com/apps/codecov. You need to configure the Github app.

@knaaptime
Copy link
Member

ah, that would help 🙃 thanks.

weird permissions issue. @sjsrey do you mind turning that on for the whole org?

@knaaptime
Copy link
Member

knaaptime commented Mar 6, 2024

pretty neat we get a global overview of the subpackage coverage... (sorry for commandeering this tread :P)

@martinfleis
Copy link
Member Author

That is cool! And @jGaboardi rocks with spaghetti on the top!

@knaaptime
Copy link
Member

as usual 🙄

...pretty impressive runner up with inequality, though...

...dont judge me on tobler. that number is BS :P

@jGaboardi
Copy link
Member

That is cool! And @jGaboardi rocks with spaghetti on the top!

The only thing that spaghetti is good at! LOL

Copy link
Member

@jGaboardi jGaboardi left a comment

Choose a reason for hiding this comment

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

LGTM

@martinfleis martinfleis merged commit 1921627 into pysal:main Mar 6, 2024
13 checks passed
@martinfleis martinfleis deleted the neighbor_distance branch March 6, 2024 21:50
@sjsrey
Copy link
Member

sjsrey commented Mar 6, 2024

ah, that would help 🙃 thanks.

weird permissions issue. @sjsrey do you mind turning that on for the whole org?

Sure, but what needs to be turned on?

@knaaptime
Copy link
Member

@sjsrey try this link https://github.com/apps/codecov and see if you can configure the app for all repositories when you get here
Screenshot 2024-03-06 at 8 12 07 PM

@sjsrey
Copy link
Member

sjsrey commented Mar 7, 2024

@sjsrey try this link https://github.com/apps/codecov and see if you can configure the app for all repositories when you get here Screenshot 2024-03-06 at 8 12 07 PM

It was already configured for all repositories:

image

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.

4 participants