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

should plot.echosounder() default to col=oceColorsViridis? #2060

Closed
dankelley opened this issue Mar 16, 2023 · 3 comments
Closed

should plot.echosounder() default to col=oceColorsViridis? #2060

dankelley opened this issue Mar 16, 2023 · 3 comments

Comments

@dankelley
Copy link
Owner

I was helping someone with echosounder data, and noticed that the default col argument was oceColorsJet(). As a test, I changed it to oceColorsTurbo(), which is quite similar but (as far as I know) superior in some ways.

But maybe we ought to default it to oceColorsViridis(), similar to imagep(). What do you think, @richardsc? My feeling is that we ought to switch, for package consistency. NOTE: the change will get noted in the docs for that function, and in NEWS, if we decide to go ahead with it.

I would vote +0.75 for switching to oceColorsViridis(), partly because I've not heard users asking about echosounder for a long time, so it's not likely this is being used a lot. And, even if so, the change from jet to viridis would be obvious and the user would likely consult the help to find the change.

I'd like to settle this prior to a CRAN release. (The time to make the change is maybe 1/3 of the time it took to write this comment, so I figure we don't have to jump on this right away ... sometime in the next few weeks would be fine for @richardsc to chime in.)

@dankelley
Copy link
Owner Author

Below is a reprex showing (top) old colours (jet), what I have at the moment in branch issue_2058_rsk (turbo) and what I am now proposing (viridis).

To my eyes, there is no question: viridis is best because it lets you see patterns more easily, without the eye being drawn to certain levels.

library(oce)
#> Loading required package: gsw
data(echosounder)
plot(echosounder, col=oceColorsJet)

plot(echosounder, col=oceColorsTurbo)

plot(echosounder, col=oceColorsViridis)

Created on 2023-03-18 with reprex v2.0.2

@richardsc
Copy link
Collaborator

Slow reply because I'm still on vacation, but I agree even just for consistency that we should be using viridis.

Ultimately the plot-echosounder,method is just an imagep() call, which should already have viridis as default, so probably this is a matter of removing the default of oceColorsJet from the col= argument.

@dankelley
Copy link
Owner Author

No worries on timing. I've done this in "develop" commit acf7fda so I'm closing this issue now.

Thanks!

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

No branches or pull requests

2 participants