Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

first cut at fixing deprecations / errors on 0.6 #536

Closed
wants to merge 17 commits into from

Conversation

sbromberger
Copy link
Owner

Branch sbromberger/compat-0.6 is designed to work on Julia 0.6. So far all tests pass, though there are deprecations in graphmatrices, randgraphs, and persistence (the latter two dealing with produce/consume stuff.)

@sbromberger sbromberger added breaking change non-deprecable change. work in progress (DO NOT MERGE) used for proofs-of-concept and other code that is ready for review but not ready for merging labels Mar 8, 2017
@codecov-io
Copy link

codecov-io commented Mar 10, 2017

Codecov Report

Merging #536 into master will increase coverage by 0.62%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #536      +/-   ##
==========================================
+ Coverage   99.13%   99.75%   +0.62%     
==========================================
  Files          57       57              
  Lines        2898     2902       +4     
==========================================
+ Hits         2873     2895      +22     
+ Misses         25        7      -18

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3481e36...1406f0e. Read the comment docs.

@sbromberger
Copy link
Owner Author

sbromberger commented Mar 10, 2017

@jpfairbanks for some reason, spectral.jl is now failing tests. I didn't do anything specific to it; just merged some other code from master in to keep this branch up to date. I tried lowering the tolerance from 1e-8 to 1e-5 but that didn't seem to do it. I also tried changing the approx function. I may have to revert that. Any ideas?

Specifically: https://travis-ci.org/JuliaGraphs/LightGraphs.jl/builds/209615638#L1033-L1035

@sbromberger sbromberger mentioned this pull request Mar 10, 2017
@jpfairbanks
Copy link
Contributor

Does this need julialang master?
I get errors on Version 0.6.0-dev.1393

INFO: Precompiling module LightGraphs.
ERROR: LoadError: LoadError: syntax: invalid type signature
 in include_from_node1(::String) at ./loading.jl:532
 in include(::String) at ./sysimg.jl:14
 in include_from_node1(::String) at ./loading.jl:532
 in include(::String) at ./sysimg.jl:14
 in anonymous at ./<missing>:2
 in eval(::Module, ::Any) at ./boot.jl:236
 in process_options(::Base.JLOptions) at ./client.jl:248
 in _start() at ./client.jl:335
while loading /home/.julia/v0.6/LightGraphs/src/core.jl, in expression starting on line 1
while loading /home/.julia/v0.6/LightGraphs/src/LightGraphs.jl, in expression starting on line 134
ERROR: Failed to precompile LightGraphs to /home/.julia/lib/v0.6/LightGraphs.ji.
 in compilecache(::String) at ./loading.jl:672
 in require(::Symbol) at ./loading.jl:453

@sbromberger
Copy link
Owner Author

I'm pretty sure it needs -alpha.

@jpfairbanks
Copy link
Contributor

I built master as of today and it worked.

There are some deprecation warnings about inner constructors, which I think are tied to changes made in the following commit.
JuliaLang/julia@0190b3a

@jpfairbanks
Copy link
Contributor

Well tests failed with GZip.jl@master:

running /home/.julia/v0.6/LightGraphs/test/persistence/persistence.jl ...
ERROR: LoadError: LoadError: UndefVarError: readall not defined
Stacktrace:
 [1] loadgraphml_mult(::GZip.GZipStream)

@sbromberger
Copy link
Owner Author

Yup. These are the combinatorial laplacian and friends.

@sbromberger
Copy link
Owner Author

It was working for me as of yesterday's build. :(

@jpfairbanks
Copy link
Contributor

So readall got renamed to readstring. I will put a commit on this branch to fix that.

What happened to Tasks and produce/consume?

[TODO]: turn this test back on before 0.6 release!
The inner constructor form:
```julia
	function NormalizedAdjacency(adjmat::CombinatorialAdjacency)
		sf = adjmat.D.^(-1/2)
		return new(adjmat, sf)
	end
 end
```
is replaced by

```julia
 function NormalizedAdjacency{T}(adjmat::CombinatorialAdjacency{T})
  sf = adjmat.D.^(-1/2)
  return NormalizedAdjacency(adjmat, sf)
 end
```

This deprecates the calling convention:

    NormalizedAdjacency{Float64}(adjmat)

in favor of

   NormalizedAdjacency(adjmat)

which is much cleaner. The {Float64} parameter was needed because
the NormalizedAdjacency(CombinatorialAdjacency{Int}(a)) needed to be
of type Float64. This promotion choice was exposed to the caller instead
of being made automatically. Now if you want to construct a
NormalizedAdjacency{Float64} from a CombinatorialAdjacency{Int} you
will have to use the default constructor and do the conversion by hand.
@jpfairbanks
Copy link
Contributor

jpfairbanks commented Mar 12, 2017

The next step is to replace produce/consume with channels.

Fixes deprecation warnings introduced in:

  JuliaLang/julia#19841

Changes an API interface:

  -function Graph(nvg::Int, neg::Int, edgestream::Task)
  +function Graph(nvg::Int, neg::Int, edgestream::Channel)

Iteration over Tasks is deprecated so now we iterate over the Channel.
@sbromberger
Copy link
Owner Author

(@jpfairbanks - I edited your list above.)

@sbromberger
Copy link
Owner Author

Where is the channels code? (Why is this checked off?)

@jpfairbanks
Copy link
Contributor

I thought I made a branch with my commits that should be a part of this PR, but I can't find it.

@sbromberger
Copy link
Owner Author

I thought I made a branch with my commits that should be a part of this PR, but I can't find it.

I thought I saw it too, and I was going to steal it. Could you (re)write it?

@jpfairbanks
Copy link
Contributor

This commit 017137a has my channels stuff. It is in this PR

@sbromberger
Copy link
Owner Author

OK, I got it (cherry picked into abstractions). I'm going to close this out since I think the plan is to make the abstractions branch the "official" 0.6 release.

@sbromberger sbromberger deleted the sbromberger/compat-0.6 branch March 29, 2017 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change non-deprecable change. work in progress (DO NOT MERGE) used for proofs-of-concept and other code that is ready for review but not ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants