Skip to content

Removing internal use of id_order in to_adjlist #510

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

Merged
merged 1 commit into from
Jan 16, 2023
Merged

Conversation

sjsrey
Copy link
Member

@sjsrey sjsrey commented Jan 16, 2023

We are going to deprecate id_order in the future, so the internal use here can be removed to address #507.

@sjsrey sjsrey added the change Change of an implementation label Jan 16, 2023
@codecov
Copy link

codecov bot commented Jan 16, 2023

Codecov Report

Merging #510 (49c69b6) into master (3a09c42) will increase coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #510     +/-   ##
========================================
+ Coverage    78.6%   78.7%   +0.1%     
========================================
  Files         122     122             
  Lines       13185   13182      -3     
========================================
+ Hits        10365   10371      +6     
+ Misses       2820    2811      -9     
Impacted Files Coverage Δ
libpysal/weights/weights.py 82.8% <ø> (-0.1%) ⬇️
libpysal/_version.py 40.7% <0.0%> (+2.7%) ⬆️

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.

Seems all good

@martinfleis
Copy link
Member

This will probably need to be revised as it broke tests in esda pysal/esda#239 and momepy pysal/momepy#461

@martinfleis
Copy link
Member

See the reproducer in pysal/momepy#461 (comment)

@martinfleis
Copy link
Member

At least the one in momepy is fixed by #511 (haven't seen that before).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change Change of an implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants