Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Update contour usage #867

Closed

Conversation

tomasaschan
Copy link
Contributor

This updates the usage of Contour to the new API introduced v0.1.0, since the old API was removed in a v0.2.0 which is about to be tagged.

I wasn't able to get the tests to pass locally, but I'm not sure it's related to this change; if it is, I don't understand how. If the tests don't pass the CI builds here, I'd be glad for any help in figuring out why.

Tomas Lycken added 2 commits August 11, 2016 11:14
This API was introduced in v0.1.0, but that version does not work
with Julia 0.5. v0.1.1 and up should.
@tomasaschan
Copy link
Contributor Author

This might also be helpful for #866.

@rsrock rsrock mentioned this pull request Aug 12, 2016
7 tasks
@tlnagy
Copy link
Member

tlnagy commented Aug 12, 2016

Running on julia v0.4.6 and using latest master of compose, the test fails (using this PR) here:

Rendering contour_function on pgf backend.
FAILED!
ERROR: LoadError: The following aesthetics are required by Geom.line to be of equal length: x, y, color, group

 in error at /usr/local/Cellar/julia/0.4.6_1/lib/julia/sys.dylib
 in assert_aesthetics_equal_length at /Users/tamasnagy/.julia/v0.4/Gadfly/src/aesthetics.jl:182
 in render at /Users/tamasnagy/.julia/v0.4/Gadfly/src/geom/line.jl:73
 in render at /Users/tamasnagy/.julia/v0.4/Gadfly/src/geometry.jl:48
 in render_prepared at /Users/tamasnagy/.julia/v0.4/Gadfly/src/Gadfly.jl:864
 in render at /Users/tamasnagy/.julia/v0.4/Gadfly/src/Gadfly.jl:803
 in draw at /Users/tamasnagy/.julia/v0.4/Gadfly/src/Gadfly.jl:913
 [inlined code] from util.jl:155
 in run_tests at /Users/tamasnagy/.julia/v0.4/Gadfly/test/runtests.jl:139
 in include at /usr/local/Cellar/julia/0.4.6_1/lib/julia/sys.dylib
 in include_from_node1 at /usr/local/Cellar/julia/0.4.6_1/lib/julia/sys.dylib
 in process_options at /usr/local/Cellar/julia/0.4.6_1/lib/julia/sys.dylib
 in _start at /usr/local/Cellar/julia/0.4.6_1/lib/julia/sys.dylib
while loading /Users/tamasnagy/.julia/v0.4/Gadfly/test/runtests.jl, in expression starting on line 183
===================================================[ ERROR: Gadfly ]====================================================

failed process: Process(`/usr/local/Cellar/julia/0.4.6_1/bin/julia --check-bounds=yes --code-coverage=none --color=yes /Users/tamasnagy/.julia/v0.4/Gadfly/test/runtests.jl`, ProcessExited(1)) [1]

========================================================================================================================

The current release of Compose is broken, but the problem is fixed on master. That might've been your problem.

@tomasaschan
Copy link
Contributor Author

The last CI build failed because the new tags for Contour hadn't been merged to METADATA.jl yet; this has happened now (JuliaLang/METADATA.jl#5901) so it might be worthwhile to restart the tests here and see what happens.

@tlnagy
Copy link
Member

tlnagy commented Aug 15, 2016

Can you close and re-open to trigger a new build?

@tomasaschan tomasaschan reopened this Aug 16, 2016
@tomasaschan
Copy link
Contributor Author

Neat, didn't know I could trigger a restart that way. Now the error seems consistent with what you saw.

I'm having some trouble testing this on my work station (running Windows and not being able to find libfontconfig) but I can hopefully get to it on my linux laptop to fix the missing pieces this weekend.

append!(contour_xs, xc)
append!(contour_ys, yc)
for _ in 1:length(xc); push!(groups, group); end
push!(levels, Contour.level(level))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this also needs to repeat like you do with groups?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged this branch in and made a fix at #870

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I also made a fix on this branch, but if you've already fixed that then this can be ignored.

@coveralls
Copy link

coveralls commented Aug 18, 2016

Coverage Status

Changes Unknown when pulling b9f1364 on tlycken:update-contour-usage into * on dcjones:master*.

@tomasaschan
Copy link
Contributor Author

tomasaschan commented Aug 18, 2016

This now fails because - nightly in Travis has changed meaning from 0.5 to 0.6; I guess a rebase might fix this, but I also think that this PR could be replaced by #870 if/when that gets merged.

Edit: merged already. Case closed :)

@shashi
Copy link
Collaborator

shashi commented Aug 18, 2016

I just enabled 0.5 on travis. Tests are running https://travis-ci.org/dcjones/Gadfly.jl/builds/153203929

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants