rework of weighted graphs API, fixes #6 and Graphs#42

[etiennedeg]

I removed all methods using internal fields from the abstract functions.
I fixed the issue #6 occurring when removing an edge. It now delete the coefficient (set the coefficient to a structural zero), instead of just zeroing it. It is done more efficiently than by calling dropzeros! and is as efficient as adding a new edge (outside for modifying a weight).
Now, the non edges are always structural zeros, so technically, we could allow setting zero weight edges. I did not permit that as it would probably be breaking, and because we can't add zeros from the weights matrix constructor, as zeros are mapped to structural zeros.

Review needed please.

[gdalle]

@etiennedeg is this still in need of review?

[etiennedeg]

yes

[InterdisciplinaryPhysicsTeam]

Hello,

Thank you for this very useful package!

We're the developers of MultilayerGraphs.jl. In trying to interface our package with every other package in the ecosystem, we incurred in the same bug as #6.

We'd really need this crucial bug to be fixed. Is there any prospective on when this will be merged? 

CC: @pitmonticone, @ClaudMor

[gdalle]

Hello,

Thank you for this very useful package!

We're the developers of MultilayerGraphs.jl. In trying to interface our package with every other package in the ecosystem, we incurred in the same bug as #6.

We'd really need this crucial bug to be fixed. Is there any prospective on when this will be merged?

CC: @pitmonticone, @ClaudMor

Alright then, just reviewed it :)
Ping @etiennedeg

[etiennedeg]

@simonschoelly Do you want a look before we merge ?

[etiennedeg]

bump @simonschoelly

[simonschoelly]

Sorry for the way too late review.

There is the possibility that this PR is breaking for some other packages, but I don't think many packages will implement their own subtype of AbstractSimpleWeightGraph - so we might be fine there. In any case I support the choice of choice of cleaning up the interface a bit more here.

Also, if I understood correctly, the rem_edge! function will now have a runtime of O(E) instead of O(log E) - this will also probably not affect to many users. But in any case we should at least increase the minor version.

I wonder, if we should check if any of the more widely used packages that have this package as a dependency will have any issues.

And at one point we probably should figure out if we should not allow edges with structural zeros - but maybe that also needs a 2.0 version.

[InterdisciplinaryPhysicsTeam]

Hello @simonschoelly,

We do have SimpleWeightedGraph.jl as a dependency in our package MultilayerGraphs.jl, which is about to receive a major update. We noticed that the rem_edge! function behaves differently in this package w.r.t. the other packages in the ecosystem of Graphs.jl, as pointed out in a comment above.

@pitmonticone @ClaudMor

[gdalle]

@simonschoelly @etiennedeg I think this has been sufficiently reviewed, should we merge to finally solve @InterdisciplinaryPhysicsTeam's problem or do we want to skim through all downstream packages first?