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

Accessibility Function #32

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Accessibility Function #32

wants to merge 16 commits into from

Conversation

lenkahas
Copy link

@lenkahas lenkahas commented Jul 7, 2021

This pull request includes all the work for the GSoC project of including competing destination estimation into SpInt module.
For more information please visit my blog https://lenkahas.com/post/gsoc.html.

@lenkahas lenkahas changed the title Initiall Accessibility Function Accessibility Function Jul 25, 2021
spint/flow_accessibility.py Outdated Show resolved Hide resolved
spint/flow_accessibility.py Outdated Show resolved Hide resolved
spint/flow_accessibility.py Outdated Show resolved Hide resolved
spint/flow_accessibility.py Outdated Show resolved Hide resolved
spint/__init__.py Outdated Show resolved Hide resolved
flow = flow.loc[:,['origin_ID', 'destination_ID','distances', 'volume_in_unipartite','dest_masses','results_all=False']]
flow['acc_uni'] = function(flow_df = flow, all_destinations=False)

self.assertEqual(flow['results_all=False'].all(), flow['acc_uni'].all())
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this does what you think it does! check what flow['results_all=False'] looks like!

You may want to use something like numpy.testing.assert_allclose or numpy.testing.assert_array_equal, which are designed to compare numpy arrays.

Copy link
Author

Choose a reason for hiding this comment

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

I think I addressed this correctly. Let me know if not :).

@ljwolf
Copy link
Member

ljwolf commented Jul 30, 2021

Looks good @lenkahas 🎉

I've got some comments/suggestions for you!

@jGaboardi
Copy link
Member

@ljwolf, It looks like @lenkahas has addressed your comments (from what I can tell). Is there anything preventing this merge?

@lenkahas
Copy link
Author

I have recently reviewed the function and needed to alter the calculation for the bipartite graph.
The test runs fine and I fixed some spelling mistakes inside the function.
Should be ready to go.

@lenkahas lenkahas closed this Dec 19, 2023
@lenkahas
Copy link
Author

I have accidentally closed this. Could this be reopened?

@ljwolf
Copy link
Member

ljwolf commented Dec 19, 2023

Super, thanks @lenkahas! I think there are still a few minor things above that need to be done before merging. Are you OK if I do these myself?

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.

5 participants