close #192

[sbromberger]

No description provided.

[vchuravy]

Maybe we could interface with FileIO?

[sbromberger]

@vchuravy see JuliaIO/FileIO.jl#46 - I'd like to figure out a way but I don't want to step on anyone else's toes to do so.

[codecov-io]

Current coverage is 100.00%

Merging #258 into master will not affect coverage as of 07d92ba

Powered by Codecov. Updated on successful CI builds.

[sbromberger]

This is a semi-breaking change: I have a deprecation in for readgraph() but that's about it. We might want to tag a new minor number. Thoughts?

cc: @jpfairbanks @CarloLucibello

[CarloLucibello]

Having something like
load("filename")
doesn't conflict with other packages, if not with julia itself?

Also, I'd like load functions to return a graph, and not a dictionary, when loading a single graph (which is by far the most common behaviour). I find the current behaviour quite annoying

[sbromberger]

The load function allows you to specify a graph name - if you do, it will return a graph instead of a dict.

[CarloLucibello]

The load function allows you to specify a graph name - if you do, it will return a graph instead of a dict.

what about returning a graph also when the name is not specified nut there is just one graph in the file?

[sbromberger]

Then load() becomes type unstable.

[CarloLucibello]

Then load() becomes type unstable.

I don't think this would impact performance

Having something like
load("filename")
doesn't conflict with other packages, if not with julia itself?

what about this?

[sbromberger]

julia> load
ERROR: UndefVarError: load not defined

I'm not so concerned about collisions with other packages.

... and type stability is a definite goal here, regardless of whether we anticipate performance problems - they will occur with this approach, and I'm not sure the benefit (a few extra characters) is worth it. You can always create your own loadone() function that will do exactly what you want:

loadone(x...) = collect(values(load(x...))[1]

[CarloLucibello]

it is conflicting with FileIO.jl, I advise against doing that, we should go with something like loadgraph. In general using such a generic name and signature for a very specialized function seems bad practice to me

[sbromberger]

They're in different namespaces. I don't really see an issue here. We don't care much when other packages reuse other functions (see, e.g., the add_* and rem_* functions that are in pretty much every graph-related (and even some non-graph-related) package).

FileIO is cool, but the reliance on a single registry that loads the first dependency that comes along seems to me to be a problem waiting to happen.

[CarloLucibello]

As a user I would expect conflict with other graph libraries, but not with generic i/o libraries. What about implementing them as extensions to FileIO in LightGraphs? like

FileIO.add_format(format"GRAPHML", (), ".graphml")

[sbromberger]

Adding to FileIO is in the plans assuming the registry issues can get resolved - as it stands I don't know how it's going to scale efficiently, particularly for common file formats. How do you handle CSVs, for example?

In any case, I'll reconsider load() / save() but I think we're solving a problem here that doesn't exist. The original methods extended write(), for example. There's nothing that would prevent us from doing that later if this became a problem and I'd rather not bikeshed on names right now - the functionality is what's most important.

[sbromberger]

I started looking at renaming but I can't come up with anything good. loadgraph() is a misnomer because it implies loading a single graph. Things like loadg() or gload() are not intuitive.

I think I'd like to just keep it load/save until we run into a problem, at which point you can reopen this and say "I told you so!"

[SimonDanisch]

I think FileIO integration would be very nice... Implementing a way to specify what IO library to prefer in FileIO is straight forward, but hasn't been on anyone's priority list high enough to actually implement it.
Considering type stability, I'm using 3 solutions so far:

# overloading the constructor:
# not very clean semantically but gets the job done quickly
x = MyType("path_to_file.myext")
# the solution that I used in my first prototype of FileIO.
# it's semantically a bit cleaner ;)
x = MyType(file"path.ext")
# And the most straightforward way, working nicely with FileIO
x = load("path.ext", MyType)

I'd vote, that the official interface should be the last option, and anything else should be implemented by calling that version.
An IO library would implement it then like that:

function load(path, typ=StandardType)
    if !(typ in SupportedTypes)
        error
    end
end