search

wcornwell / earthtones

Derive a Color Palette from a Particular Location on Earth
Other
31 stars 1 forks source link

remove duplication functionality in plot_satellite_image_and_pallette() #3

Closed mitchest closed 8 years ago

mitchest commented 8 years ago

It would be good to get rid of plot_satellite_image_and_pallette() and simply control it through the main get_earthtones() and plot function. I reckon there should be an argument in get_earthtones() called say plotSatellite= and have it default to FALSE. If it's true, then you append the map object to the attributes of out.col (that get_eathtones() returns). Then in plot.palette you simply havea conditional that detects if the map attribute exists, and plot(map) if it does. What do you think @wcornwell ?

wcornwell commented 8 years ago

good point. I think probably all users will want to see the plot first so I'd rather have it default to TRUE, then if they (later) incorportate it into a script they could change it to false. I have a look at this.

wcornwell commented 8 years ago

Got some of this done, but not everything. What do you think?

mitchest commented 8 years ago

Yer nice, I was going to say if you think the map plot should be default then just make it a list out. It's not strictly the way you should use a class, but you could make both the returns of class 'palette' and just make the S3 print detect whether $map exists - i don't think it's worth having two classes and two S3 funcs at this stage. The only disadvantage of a list out of get_earthtones() is that you can't directly use the vector of colours, but I can't see that ever really being an issue =p