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

introduce rgbplot #339

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

introduce rgbplot #339

wants to merge 2 commits into from

Conversation

maxfreu
Copy link
Contributor

@maxfreu maxfreu commented Nov 29, 2022

Hi! I've just put my plotting code into a dedicated rgbplot function, which should make plotting rasters as rgb images convenient :) I've also modified the skipmissing stuff, because I had errors when the missingval was nothing.

@rafaqz
Copy link
Owner

rafaqz commented Nov 29, 2022

Amazing!

One question: is there a reason you don't use a plot(raster; rgb=true) ?

There is quite a bit if code duplication here, and the RGBPlot wrapper is a little unusual. Using an rgb keyword could solve that.

@maxfreu
Copy link
Contributor Author

maxfreu commented Nov 30, 2022

I’m just not that familiar with writing plot recipes. I can try to change it.

@rafaqz
Copy link
Owner

rafaqz commented Nov 30, 2022

Yes its a little mysterious, but there is a plotattributes object in scope in recipes functions that has the keyword arguments in it:

So something like this should work:

isrgb = get(plotattributes, :rgb, false)

And

bands = get(plotattributes, :bands, 1:3)

@maxfreu
Copy link
Contributor Author

maxfreu commented Nov 30, 2022

Ok, I'll give it a try when I find the time again. How does one properly document the behaviour for plot recipes?

@rafaqz
Copy link
Owner

rafaqz commented Nov 30, 2022

I don't really know how to document them! in all my packages the plotting is very poorly documented

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.

2 participants