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

NonEmpty AdjancencyIntMaps continued #280

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

zyklotomic
Copy link

Continuing off of #250, and with the additional goal of adding support for SCC decomposition for AdjacencyIntMaps

@zyklotomic zyklotomic changed the title NonEmpty AdjancencyIntMaps + SCC Algo NonEmpty AdjancencyIntMaps continued + SCC Algo Oct 31, 2021
@zyklotomic
Copy link
Author

The commit I just pushed was for the SCC algorithm that I hoped to have for the IntMap version of graphs. I have yet to test it.

If I understand correctly, what still needed to be done from the original PR was the test suite right? I will also need to add tests for IntMap SCC. I will look into that next! :D

@snowleopard
Copy link
Owner

@zyklotomic Hi there, thanks for taking this over! My preference would be to finish the NonEmpty PR first, and then implement SCC as a separate PR. It would just make it easier for review. Does that work for you?

@zyklotomic
Copy link
Author

Yes I can do that, my apologies! I don't think I can directly work off of the old PR since I don't have push access to jitwit's branch. Should I create a new PR for NonEmpty, and rename this to being the SCC PR?

@snowleopard
Copy link
Owner

snowleopard commented Nov 3, 2021

I think it's fine to keep working on NonEmpty in this PR. If you could just undo the SCC-related changes for now that would be great. Of course, please take care not to lose the SCC changes since we are going to need them eventually!

@zyklotomic zyklotomic changed the title NonEmpty AdjancencyIntMaps continued + SCC Algo NonEmpty AdjancencyIntMaps continued Nov 3, 2021
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.

None yet

3 participants