dankelley / oce

R package for oceanographic processing
http://dankelley.github.io/oce/
GNU General Public License v3.0
145 stars 42 forks source link

Make "decimate" for imagep() an option that can be set #2263

Closed richardsc closed 1 week ago

richardsc commented 1 week ago

I love the addition of the decimate= arg in imagep(), but at times I'd like to be able to turn it off by default -- either because I know I want the highest resolution plot, or because I don't want the warnings continually dumping to the console.

Proposal: Can we make a new option associated with oce called something like oceDecimateImagep, set as default to TRUE, and then accordingly change imagep() so that the default value is set to that arg instead of the current of TRUE.

What do you think?

dankelley commented 1 week ago

Sure, we can definitely do that. The only (slight) negative is that then the R files will not be reproducible, without also including the startup file. But we already have that with eos, so that's nothing new.

I can do it this morning, if you like. It's ten minutes.

richardsc commented 1 week ago

Right. But I think it's like other oce options that we have, e.g. oceDrawTimeRange, that are set as default to TRUE, but can be changed at the script level, which is reproducible.

dankelley commented 1 week ago

Yup, I've done it but not pushed to GH yet, until a local build-test works.

On Nov 21, 2024, at 7:33 AM, Clark Richards @.***> wrote:

Right. But I think it's like other oce options that we have, e.g. oceDrawTimeRange, that are set as default to TRUE, but can be changed at the script level, which is reproducible.

— Reply to this email directly, view it on GitHub https://github.com/dankelley/oce/issues/2263#issuecomment-2490875310, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAYJDIVINP5ENQJJTUI2KL2BXAHFAVCNFSM6AAAAABSF4JQLWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOJQHA3TKMZRGA. You are receiving this because you commented.

dankelley commented 1 week ago

Done in "develop" as shown below. NOTE: I think the first vignette needs to have a section called .Rprofile startup file or something like that. But first I need to check that we don't already have that, someplace. Anyway, that is not entirely related to this issue. @richardsc please consider closing this issue, after checking and further thought.

➜ git/oce git:(develop) git log -2 commit 68312f8d6e171850e0c51341197da9256d975219 (HEAD -> develop, origin/develop, origin/HEAD) Author: dankelley kelley.dan@gmail.com Date: Thu Nov 21 08:15:49 2024 -0400

rebuild webpage

commit c42ab9cc1bddf62e79cefc0527d65328e3487399 Author: dankelley kelley.dan@gmail.com Date: Thu Nov 21 07:57:45 2024 -0400

imagep() can now default 'decimate' from .Rprofile
dankelley commented 1 week ago

Oh, I see that we have a whole vignette on setting defaults. I'll add this there.

dankelley commented 1 week ago

commit a21a18622c5fcc4057a683b8026de140640d2202 Author: dankelley kelley.dan@gmail.com Date: Thu Nov 21 08:33:30 2024 -0400

document new addition to .Rprofile
richardsc commented 1 week ago

Thanks, this looks great!

My only comment would be on the wording in the help page about the use of option() in a .Rprofile. I don't think we need to be that specific, as of course the option() function can be used anywhere -- either in a .Rprofile or within a script or Rmd.

I could make a PR, but it might be easier for you to just update. In which case, I'd propose the following (also note the change from "three options" to "four options":

an item that controls whether the image will be decimated before plotting, in four possible cases. Note that the default value of TRUE can be overridden by using options() as e.g. options(oceImageDecimate = FALSE) in a ~/.Rprofile startup file or locally within a script or session.

dankelley commented 1 week ago

I'll make the change and push to GH when the local build-check is done (like 20 minutes)