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

Add exponential kernel to Graph #761

Merged
merged 1 commit into from
Aug 15, 2024
Merged

Add exponential kernel to Graph #761

merged 1 commit into from
Aug 15, 2024

Conversation

knaaptime
Copy link
Member

No description provided.

Copy link

codecov bot commented Aug 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.4%. Comparing base (27776c3) to head (d52d684).
Report is 39 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #761   +/-   ##
=====================================
  Coverage   85.4%   85.4%           
=====================================
  Files        150     150           
  Lines      15975   15989   +14     
=====================================
+ Hits       13641   13655   +14     
  Misses      2334    2334           
Files Coverage Δ
libpysal/graph/_kernel.py 82.4% <100.0%> (+0.4%) ⬆️
libpysal/graph/tests/test_kernel.py 99.1% <100.0%> (+<0.1%) ⬆️

... and 2 files with indirect coverage changes

@martinfleis martinfleis changed the title exponential kernel Add exponential kernel to Graph Aug 6, 2024
@@ -106,6 +112,7 @@ def _kernel(
- gaussian:
- bisquare:
- cosine:
- exponential:
Copy link
Member Author

Choose a reason for hiding this comment

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

should these all probably have formulas here?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally

Copy link
Member

@ljwolf ljwolf left a comment

Choose a reason for hiding this comment

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

Looks great to me!

@knaaptime
Copy link
Member Author

no way to bound it at the bandwidth if you want though 🤷‍♂️. would require another argument

@martinfleis
Copy link
Member

Thinking about this more, I am actually not sure if I like passing alpha as bandwidth. It is inconsistent with the rest of the kernels then. Also, distance band has both bandwidth and alpha arguments (though not used together), so we shall aim for some degree of consistency between these.

@knaaptime knaaptime merged commit bef1e81 into pysal:main Aug 15, 2024
12 checks passed
@knaaptime knaaptime deleted the exp branch August 15, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants