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

Vertex/Arc ID sorting bug in Network.split_arcs() #535

Merged
merged 20 commits into from
Oct 16, 2020

Conversation

jGaboardi
Copy link
Member

This PR addresses #526.

The were 2 sorting issues happening here.

  1. The order of vertex IDs comprising an arc ID was causing issues during the split. When a network object is split some vertices are retained and some new vertices are added. This was causing issues with iterating over arcs.
  2. Arc IDs were not being re-sorted following conversion back from a set after the network was split. When the arcs are fed into libpysal.weights.W it expects sorted IDs, this is what was causing all the counts all over the place.

@jGaboardi
Copy link
Member Author

All tests are passing locally (macOS, Python 3.8) and passing CI for Ubuntu, Python 3.8, but then failing out on Ubuntu, Python 3.6. Appears to be an indexing/sorting discrepancy at first glance.

@jGaboardi
Copy link
Member Author

jGaboardi commented Oct 13, 2020

Passing:

OS/Python 3.6 3.7 3.8
macOS
Ubuntu
Windows

Python 3.6 on Windows failing due to an issue with fiona upstream. The issue started with fiona build win-64/fiona-1.8.17-py36hdef4c2b_0.tar.bz2 (see https://anaconda.org/conda-forge/fiona/files) and persists in win-64/fiona-1.8.17-py36hdef4c2b_1.tar.bz2.

@jGaboardi
Copy link
Member Author

The set/indexing issue is resolved, but an issue with fiona upstream is causing failures on Windows builds.

@codecov
Copy link

codecov bot commented Oct 15, 2020

Codecov Report

Merging #535 into master will increase coverage by 0.3%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #535     +/-   ##
========================================
+ Coverage    97.6%   97.9%   +0.3%     
========================================
  Files           3       3             
  Lines        1136    1143      +7     
  Branches      284     286      +2     
========================================
+ Hits         1109    1119     +10     
+ Misses          6       4      -2     
+ Partials       21      20      -1     
Impacted Files Coverage Δ
spaghetti/network.py 97.8% <100.0%> (+0.4%) ⬆️

@jGaboardi
Copy link
Member Author

The current commit will fail, but we'll merge anyway in the hopes the fiona build issue with Python 3.6 on Windows will be resolved.

@jGaboardi jGaboardi merged commit f306eb5 into pysal:master Oct 16, 2020
@jGaboardi jGaboardi deleted the split_bug branch October 16, 2020 23:36
@jGaboardi jGaboardi changed the title [WIP] Vertex/Arc ID sorting bug in Network.split_arcs() Vertex/Arc ID sorting bug in Network.split_arcs() Oct 22, 2020
@jGaboardi jGaboardi removed the WIP label Oct 22, 2020
jGaboardi added a commit that referenced this pull request Oct 22, 2020
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.

1 participant