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

fix colormap error preventing zcol values when using x0/x1/col0/col1 #1644

Merged
merged 1 commit into from Jan 8, 2020

Conversation

richardsc
Copy link
Collaborator

@richardsc richardsc commented Jan 8, 2020

Fixes a bug in the colormap function where zcol is assigned "black" no matter whether z was provided or not. Also removes two redundant lines.

@richardsc richardsc merged commit 13d17d6 into develop Jan 8, 2020
0 of 4 checks passed
@richardsc
Copy link
Collaborator Author

@richardsc richardsc commented Jan 8, 2020

I noticed that there is a comment in the 4th example in ?colormap that says that it is broken. I think that might actually be fixed now ... (though there is a problem with the blend argument, which is supposed to be between 0 and 1, and also not sure if yelloe is a typo).

Existing example (which is not run):

cm <- colormap(x0=c(-8000, -4000,   0,  100),
                    x1=c(-4000,     0, 100, 5000),
                    col0=c("violet","blue","white","tan"),
                    col1=c("blue","white","tan","yelloe"),
                    blend=c(100, 8, 0))
     lon <- topoWorld[['longitude']]
     lat <- topoWorld[['latitude']]
     z <- topoWorld[['z']]
     imagep(lon, lat, z, breaks=cm$breaks, col=cm$col)
     contour(lon, lat, z, levels=0, add=TRUE)
     message("colormap() example 4 is broken")

Possible new example:

cm <- colormap(x0=c(-8000, -4000,   0,  100),
                    x1=c(-4000,     0, 100, 5000),
                    col0=c("violet","blue","white","tan"),
                    col1=c("blue","white","tan","yellow"))
     lon <- topoWorld[['longitude']]
     lat <- topoWorld[['latitude']]
     z <- topoWorld[['z']]
     imagep(lon, lat, z, breaks=cm$breaks, col=cm$col)
     contour(lon, lat, z, levels=0, add=TRUE)

image

@richardsc richardsc deleted the colormap branch Jan 8, 2020
@richardsc
Copy link
Collaborator Author

@richardsc richardsc commented Jan 8, 2020

Leaving this open for now for @dankelley review.

@richardsc richardsc self-assigned this Jan 8, 2020
@richardsc richardsc requested a review from dankelley Jan 8, 2020
@dankelley
Copy link
Owner

@dankelley dankelley commented Jan 8, 2020

Thanks, Clark, this looks great. I also fixed up the spelling error in ex4 of the doc, and also an error where I was giving merge as a triplet, instead of a numeric between 0 and 1, which is what the doc demands. This build is clean in appveyor, and looks clean so far in the slower travisCI.

HINT: the appveyor is the second icon on the README as rendered on GH. It usually finishes its test in under half an hour. It seems to be better than travisCI, in that it seldom dies because the test software is missing some system library or other.

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

Successfully merging this pull request may close these issues.

None yet

2 participants