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

BUG: fix Graph.to_W #551

Merged
merged 1 commit into from
Aug 22, 2023
Merged

BUG: fix Graph.to_W #551

merged 1 commit into from
Aug 22, 2023

Conversation

martinfleis
Copy link
Member

I've noticed that when ids are not ordered, they get ordered within groupby which messes up the conversion to W. This seems to fix it.

@martinfleis martinfleis self-assigned this Aug 22, 2023
@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #551 (683b3ce) into geographs (bd228c7) will increase coverage by 0.1%.
The diff coverage is 100.0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           geographs    #551     +/-   ##
===========================================
+ Coverage       81.1%   81.1%   +0.1%     
===========================================
  Files            127     127             
  Lines          14549   14560     +11     
===========================================
+ Hits           11794   11814     +20     
+ Misses          2755    2746      -9     
Files Changed Coverage Δ
libpysal/graph/base.py 98.2% <100.0%> (+<0.1%) ⬆️
libpysal/graph/tests/test_base.py 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

@jGaboardi
Copy link
Member

LGTM. Let's merge after Windows passes.

@jGaboardi
Copy link
Member

(as a side note, I've never even heard of pandas.factorize(). You really do learn something new every day!)

@jGaboardi jGaboardi merged commit 65f5c25 into pysal:geographs Aug 22, 2023
9 checks passed
@martinfleis martinfleis deleted the geog_cont branch August 22, 2023 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants