pysal / pointpats

Planar Point Pattern Analysis in PySAL
https://pysal.org/pointpats/
BSD 3-Clause "New" or "Revised" License
80 stars 26 forks source link

COMPAT: ensure we pass numpy array to cKDtree and combat with sklearn 1.3.0 #107

Closed martinfleis closed 1 year ago

martinfleis commented 1 year ago

It seems that the DataFrame input to query worked but was never actually supported. This ensures we always cast it to a numpy array.

Closes #106

We should probably cut a patch release with this as any env with new scipy will have this issue.

martinfleis commented 1 year ago

mmm. New failures are caused by fresh scikit-learn 1.3.0 where KDTree.valid_metrics is no longer an attribute but a method. Will look into that.

martinfleis commented 1 year ago

Apparently KDtree.valid_metrics was not considered public before? And they now added KDtree.valid_metrics() in https://github.com/scikit-learn/scikit-learn/pull/25482 See https://github.com/scikit-learn/scikit-learn/issues/26742

codecov[bot] commented 1 year ago

Codecov Report

Merging #107 (8d616ca) into main (44ce06b) will increase coverage by 0.18%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #107      +/-   ##
==========================================
+ Coverage   52.38%   52.56%   +0.18%     
==========================================
  Files          12       12              
  Lines        1844     1851       +7     
  Branches      316      317       +1     
==========================================
+ Hits          966      973       +7     
  Misses        821      821              
  Partials       57       57              
Impacted Files Coverage Δ
pointpats/geometry.py 78.57% <100.00%> (+0.93%) :arrow_up:
pointpats/pointpattern.py 70.05% <100.00%> (ø)
martinfleis commented 1 year ago

sorry for a self-merge but this was completely blocking any testing now and I wanted to work on #109 and #108

martinfleis commented 1 year ago

Note to myself that we may need to eventually revert the sklearn compat part because it is a regression in upstream and they will restore the original behaviour (property, not a method).