JuliaAttic / OldGraphs.jl

Working with graphs in Julia
Other
205 stars 81 forks source link

Order of parameters #110

Open deinst opened 10 years ago

deinst commented 10 years ago

In most methods, the graph is listed last, but in others it is listed first. There are probably good reasons for each particular case, but I've ordered the parameters incorrectly enough times to wish that it was consistent. My preference would be to have the graph as the first parameter, but that is probably my Java/C++ background talking, all I really want is to not have to think about it.

lindahua commented 10 years ago

The ordering was following the C++ Boost Graph Library: http://www.boost.org/doc/libs/1_55_0/libs/graph/doc/graph_concepts.html

mlubin commented 10 years ago

I think it would be a reasonable convention to put the graph first.

deinst commented 10 years ago

@lindahua That explains it. I see that you have cloned the BGL quite faithfully, I probably should have been more aware of that. I'm still not sure that it makes sense. Mostly the algorithms have the graph first, while the construction methods have it last. (I suspect that the parameter order is a legacy of the attempt to get around C++'s lack of default parameters). At least Julia is a couple of orders of magnitude more pleasant about its error messages than boost. I can live with it the way it is for now.

lindahua commented 10 years ago

Personally, I like putting the graph first. (The Boost order actually gets quite unnatural to me).

I don't mind changing the order if someone take the lead to do that.

mlubin commented 10 years ago

Deprecation warnings would be greatly appreciated.

deinst commented 10 years ago

Very good. I may look at this this weekend. If I do change things, I'll keep the old interfaces and complain appropriately. If we do change though, it would be better sooner than later.

Could you commit kmsquire's #106? I'd rather not have to merge things by hand.

lindahua commented 10 years ago

@kmsquire has merged #106.

deinst commented 10 years ago

This will have to be on hold for a while. Keeping the old and new parameter orders drives the julia dispatcher completely nuts. As soon as that is fixed, I'll try again.