davecom / SwiftGraph

A Graph Data Structure in Pure Swift
Apache License 2.0
754 stars 79 forks source link

Directed pseudo forest #78

Closed ferranpujolcamins closed 4 years ago

ferranpujolcamins commented 4 years ago

In this PR I add a subclass of UniqueElementsGraph with the property that vertices have at most one outgoing edge [1].

I also introduce SwiftCheck for property testing.

[1] https://en.wikipedia.org/wiki/Pseudoforest#Directed_pseudoforests

ferranpujolcamins commented 4 years ago

@davecom Now that xcode supports swift packages, do you think we can just get rid of the xcodeproj?

davecom commented 4 years ago

Sorry I've been out of touch. I'm going to take a look at this over the weekend. I don't want to get rid of the Xcode project because it has the Mac sample app and it also just makes it easy for people unfamiliar with SPM/Cocoapods to get started.

ferranpujolcamins commented 4 years ago

I understand. I'll add the SwiftCheck package on the proj also then.

ferranpujolcamins commented 4 years ago

I'm done with the final touches, the PR is ready from my side. Let me know what you think.

ferranpujolcamins commented 4 years ago

Found a bug on the graph validation code. I'm writing tests now.

davecom commented 4 years ago

Hi @ferranpujolcamins, Thanks for putting this together. There's good stuff here, but I don't want to add SwiftCheck as a dependency (which in turn has another package as a dependency, which in turn has yet another package as a dependency). We currently have no dependencies that are not shipped with Swift, and I'd like to keep it that way. Especially since SwiftGraph is part of the Swift source compatibility suite, but also because I don't want to start being dependent on other projects as Swift continues to evolve.

Let's put the generators into the main project. Our project is not large enough that we need to start splitting it up into subdivisions.

Can you also add some comments in. I don't know what some of these are supposed to do.

Thanks, Dave

ferranpujolcamins commented 4 years ago

Hi Dave, i understand.

However the Generators also depend ln Swiftcheck.

I will strip the property tests from the PR and leave only DirectedPseudoForest. Then I will add a bunch of normal tests for it.

I will create a new repo with the property tests and Generators. Then I will add a reference to the new repo on the readme of SwiftGraph.

I think property tests are important, but I don’t want to create a forkof SwiftGraph, so I think this is a good compromise.

Would this work for you?

On Mon, 9 Mar 2020 at 07:52, David Kopec notifications@github.com wrote:

Hi @ferranpujolcamins https://github.com/ferranpujolcamins, Thanks for putting this together. There's good stuff here, but I don't want to add SwiftCheck as a dependency (which in turn has another package as a dependency, which in turn has yet another package as a dependency). We currently have no dependencies that are not shipped with Swift, and I'd like to keep it that way. Especially since SwiftGraph is part of the Swift source compatibility suite, but also because I don't want to start being dependent on other projects as Swift continues to evolve.

Let's put the generators into the main project. Our project is not large enough that we need to start splitting it up into subdivisions.

Can you also add some comments in. I don't know what some of these are supposed to do.

Thanks, Dave

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/davecom/SwiftGraph/pull/78?email_source=notifications&email_token=ABRBYTYNYJDPULJH64QLZY3RGSN3HA5CNFSM4LATQW7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOF3IAQ#issuecomment-596358146, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRBYTYSWAT4IEOJCJAMRHTRGSN3HANCNFSM4LATQW7A .

ferranpujolcamins commented 4 years ago

@davecom Gentle ping.

I will strip the property tests from the PR and leave only DirectedPseudoForest. Then I will add a bunch of normal tests for it.

Do you think this would fit SwiftGraph?

davecom commented 4 years ago

@ferranpujolcamins Sorry for the late reply. I think your plan is good!

ferranpujolcamins commented 4 years ago

I'm not finding the time to do this. I'm closing the PR by now. My apologies.