Skip to content
This repository was archived by the owner on Apr 26, 2019. It is now read-only.

search: add topological sort helper function#80

Merged
kortschak merged 1 commit intomasterfrom
sort
Jun 9, 2015
Merged

search: add topological sort helper function#80
kortschak merged 1 commit intomasterfrom
sort

Conversation

@kortschak
Copy link
Member

Fixes #78.

The naming might be contentious. The thinking is that TarjanSCC and this will end up in the structure package (that I am now thinking should be called topofor shortness) - the contents of which might look something like it does in api-explore.

We could just do a DFS toposort, but I think the value of being able to provide a reason for failure is worth any additional computational cost. The maintenance cost is 0, since we already need to look after TarjanSCC anyway.

@gonum/developers PTAL

@kortschak kortschak force-pushed the sort branch 9 times, most recently from c0747d2 to 6ad639a Compare June 8, 2015 10:37
@vladimir-ch
Copy link
Member

Code reuse and the information returned from Sort() are a good reason to use TarjanSCC. It would be really nice if this eventually ended up in a "topo" package.

LGTM

@kortschak
Copy link
Member Author

I have made minor changes - explicit statement of sort order and reversal of the order provided by TarjanSCC. I'll merge, but let me know if you disagree and I'll revert those.

@kortschak kortschak merged commit a1f37b2 into master Jun 9, 2015
@kortschak kortschak deleted the sort branch June 9, 2015 05:24
@UserAB1236872
Copy link
Contributor

Already merged, but seems good to me too

On Mon, Jun 8, 2015 at 10:11 PM, Dan Kortschak notifications@github.com
wrote:

Merged #80 #80.


Reply to this email directly or view it on GitHub
#80 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants