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.

Sign up for GitHub

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

Jump to bottom

fix indexing issue with adjlist construction #528

Merged
merged 3 commits into from
May 24, 2023
Merged

fix indexing issue with adjlist construction #528

merged 3 commits into from
May 24, 2023

Conversation

ljwolf
Copy link
Member

@ljwolf ljwolf commented May 24, 2023

see my comment in pysal/esda#245 for rationale and the fix implemented here.

@ljwolf ljwolf requested review from sjsrey and martinfleis May 24, 2023 19:34
@ljwolf ljwolf added bug functionality that: returns invalid, erroneous, or meaningless results; or doesn't work at all. weights labels May 24, 2023
@jGaboardi
Copy link
Member

Possibly related to and fix for pysal/spopt#346

@codecov
Copy link

codecov bot commented May 24, 2023 •
edited
Loading

Codecov Report

Merging #528 (40e71bf) into main (d5ea8f9) will increase coverage by 0.1%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #528     +/-   ##
=======================================
+ Coverage   79.9%   79.9%   +0.1%     
=======================================
  Files        113     113             
  Lines      13130   13135      +5     
=======================================
+ Hits       10486   10500     +14     
+ Misses      2644    2635      -9
Impacted Files Coverage Δ
libpysal/weights/tests/test_adjlist.py 98.5% <100.0%> (+0.1%) ⬆️
libpysal/weights/weights.py 83.2% <100.0%> (ø)

... and 1 file with indirect coverage changes

martinfleis
martinfleis approved these changes May 24, 2023
View reviewed changes
Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

Thanks!

We really need to get rid of ordering... It is a can of worms and I'm sure this is not the only bug that is in there.

@ljwolf ljwolf merged commit 8fa39a8 into pysal:main May 24, 2023
ljwolf referenced this pull request May 25, 2023
@sjsrey @knaaptime @martinfleis
Handle set_index properly in to_adjlist (#511)
969039d 
* Handle set_index properly in to_adjlist

* Add option to sort/unsort return from to_adjlist

* Update libpysal/weights/tests/test_adjlist.py

Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>

Co-authored-by: eli knaap <ek@knaaptime.com>
Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>
This was referenced May 25, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinfleis martinfleis martinfleis approved these changes

@sjsrey sjsrey Awaiting requested review from sjsrey

Assignees
No one assigned
Labels
bug functionality that: returns invalid, erroneous, or meaningless results; or doesn't work at all. weights
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ljwolf @jGaboardi @martinfleis