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

Informative warning for shortest path unsigned integer #169

Merged
merged 4 commits into from
Jul 25, 2023

Conversation

PyMap
Copy link
Collaborator

@PyMap PyMap commented Aug 20, 2021

From #166 and #168 we know that unconnected nodes in net object return the unsigned 4294967.295 integer value for shortest path distance calculation. In order to identify the external indexes of these nodes I included informative warnings for single integer and list of integers between origin and destinations.

Non connected idx can be found in #168, the data and script to reproduce/test warnings in both shortest_path_length/shortest_path_lengths methods can be found here

To run the script:

python pdna_path.py --origin 1599 5062  --destin 557 6235 

Identify external nodes idx for unsigned shortest path distance
@github-actions
Copy link

Test coverage is 89%

Copy link
Contributor

@federicofernandez federicofernandez left a comment

Choose a reason for hiding this comment

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

Great improvement. Minor comment: it would be great to assign 4294967.295 to a constant with a meaningful name to improve future readability of the code.

@PyMap
Copy link
Collaborator Author

PyMap commented Aug 23, 2021

@federicofernandez thanks for the feedback! You suggest something like:

invalid_result = 4294967.295

if shortest_path distance == invalid_result:
      warning.warn ...

I know that "invalid result" wouldn't be creative enough, just asking to catch the idea

@smmaurer smmaurer merged commit 9616c2c into dev Jul 25, 2023
6 of 21 checks passed
@smmaurer smmaurer deleted the issue/shpath_168 branch July 25, 2023 21:44
@smmaurer smmaurer mentioned this pull request Jul 25, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants