`TScontrol` in `plot,argoFloats-method` should have an option for plot type (e.g. points, lines, etc)

[richardsc]

This is very low priority, but just something that I thought would be nice as it's often something that I try and do -- that is, to plot each cycle in a TS plot as a line (perhaps colored by cycle), rather than a scatter of points.

Note that currently, one can do the "normal" thing of just passing type="l", but that gives a line plot where each cycle gets connected to the ones before/after and looks like a mess:

## not a reprex
plot(d, which='TS', type='l')

[richardsc]

A potential argument format could be something like:

plot(d, which='TS', TScontrol=list(colByCycle=1:10, type="l"))

which would produce a TS plot with lines for each profile, colored by cycle.

I'll look into this.

[richardsc]

Just taking notes as I look at the code and think about this. Other devs can just ignore 😄

Hm, this is a little tricky to do as easily as I thought, because the current implementation extracts all the S, T, and p data from the argos object as one long vector each.

The way I've done similar in the past is to do the extraction so that each profile has an NA in between them (which will cause a break when the lines are plotted), but that's tricky here because it would require making an if case that is separate from the main one, e.g.

                  oce::plotTS(ctd,
                              cex=cex,
                              bg=bg,
                              col=col,
                              pch=pch,
                              mar=par("mar"), mgp=par("mgp"), eos=eos,
                              type=if (is.null(type)) "p" else type, ...)

[dankelley]

Not sure I understand what d is, in the initial comment, but the following code and result might be relevant.

library(oce)
library(argoFloats)
argos <- getIndex() |> subset(ID="6990512") |> getProfiles() |> readProfiles()
Slim <- argos[["salinity"]] |> sapply(range) |> range()
Tlim <- argos[["temperature"]] |> sapply(range) |> range()
legend <- NULL
png("argosTS.png")
for (i in seq_len(argos[["length"]])) {
    plotTS(argos[[i]], Slim=Slim, Tlim=Tlim, add=i!=1, col=i, pch=20, cex=0.5)
    legend <- c(legend, gsub(".*/", "", argos[["filename"]][i]))
}
legend("topleft", col=seq_len(argos[["length"]]), legend=legend,
    bg="white", pch=20, pt.cex=0.5)

[dankelley]

Sorry, didn't read the title of the issue. Sheesh, somebody ought to take away my github license.

library(oce)
library(argoFloats)
argos <- getIndex() |> subset(ID="6990512") |> getProfiles() |> readProfiles()
Slim <- argos[["SA"]] |> sapply(range) |> range()
Tlim <- argos[["CT"]] |> sapply(range) |> range()
legend <- NULL
png("argoTS.png")
for (i in seq_len(argos[["length"]])) {
    plotTS(argos[[i]], Slim=Slim, Tlim=Tlim, add=i!=1, col=i, pch=20, cex=0.5, type="l")
    legend <- c(legend, gsub(".*/", "", argos[["filename"]][i]))
}
legend("topleft", col=seq_len(argos[["length"]]), legend=legend,
    bg="white", lwd=par("lwd"))

[dankelley]

Clark, I wonder why the 001 and 001D are so very different near the top. I guess the 001D was on the deployment, at time t0 say, and 001 was on the first rising, at time t0+10 days? Easy enough to find out, I guess. I put in a thing to spit out time and location, as below. So 001 was about 3 days after 001D, but really very near it. Maybe it was near a front or something. Irrelevant to the actual issue, I know, sorry,

001: 2023-03-09 22:47:40 -23.0107233333333E 11.455475N
001D: 2023-03-07 03:28:00 -22.98165E 11.4655366666667N
002: 2023-03-19 17:28:40 -23.0161166666667E 11.3099383333333N

[richardsc]

Oh, nice -- that's a more elegant version of what I was messing with. I have to say, I had to REALLY squint at this argument for about 30 seconds before I realized what was going on:

    add=i!=1

It reminds me of this confusing expression:

> !11 %in% 1:10
[1] TRUE

becase I feel like it should have to be written:

> !(11 %in% 1:10)
[1] TRUE

but it works.

[richardsc]

As for the issue, what do you think about sliding a version of your code into the plot,argoFloats-method?

[dankelley]

1

Re !x %in% y The way I remember that the %in% takes precedence is that ! is meant for logicals. So, for example, if x and y are characters, then the evaluating !x first would mean negating a string, which is an error. So, for it to make sense in general, regardless of types, the %in% must take precedence (i.e. be performed first). Having said that, I write !(...) in code I expect other folks to read, and only use the other form when it's for myself.

2

Re slide-in, yes, I think so. We could maybe ask @j-harbin to do it, depending on other projects, funding, etc. (I like asking junior folks to do work.)

One thing: where I wrote e.g.

Slim <- ...[["salinity"]]...

the code should have

Sname <- if ("gsw" == getOption("oceEOS", "gsw")) "SA" else "salinity"
Slim <- ...[[Sname]]...

and the same for temperature. That way, the limits will work regardless of which EOS the user has set. (I have one EOS as the default on my laptop, and the other on my defunct desktop.)

[dankelley]

Oh, sorry, one more thing: for a package, I don't think |> should be used because it requires a fairly recent R to work. And some folks might not keep their R updated on a yearly timescale.

It's easy to recode, by using quote() on some code:

> quote(a|>b()|>c())
c(b(a))

[richardsc]

In regard to the differences with the descending you noticed, that's actually one of the reasons I was looking at this float -- it has an RBR CTD, so the thermal inertia errors should be different for up vs down when going through a strong temperature gradient (e.g. the base of the mixed layer).

See:

for (i in 1:length(argos[['argos']])) {
    plotProfile(argos[['argos']][[i]], xtype='salinity', plim=c(100, 0), Slim=Slim, add=i!=1, col=i)
}

[richardsc]

I don't want to distract @j-harbin too much from other things she's working on right now, so I'll poke at this when I have a chance 😄

[dankelley]

Here's something to watch if the build is slow (apologies to E. Hemingway) https://youtu.be/4PaWFYm0kEw?t=1243

[github-actions[bot]]

The Stale-bot has marked this issue as Stale, because no new comments have been added in the past 30 days. Unless a comment is added within the next 7 days, the Stale-bot will close the issue. The purpose of these automated actions is to prevent the developers from forgetting about unattended tasks. Note that adding a "pinned" label will turn this action off for a given issue.

[richardsc]

In classic Clark-fashion, I opened this, suggested @dankelley make some additions based on his example, and then we both went down a bunch of weird rabbit holes about the data and forgot about it. I think this is still a good addition, and I would even be happy to do it, when I get a chance.

So, I'm leaving it open :smile:

[dankelley]

I've added my name to the assignees list. I might like some plots like this for classes. (Yup, at this time of year I start to prioritize things that would be nice for classes.)

[dankelley]

I'll do my tests in a new branch called 591_plot_TS_control.

[dankelley]

I've started on this. The name colByCycle is not the best. For example, the user might want to draw all black, with type="l", and without the joining lines. So it's not really colouring by cycle, it's drawing by cycle.

I don't know how long colByCycle has been in the code, but I think the argument against that name is strong enough that the code ought to detect it and say to use another word instead.

Indeed, when I started playing with this today, I thought colByCycle was a logical value. It was only when I looked in the docs that I saw it was a list of cycles. I think what would be nice would be something like

plot(a, which="TS", TSControl=list(separateCycles=TRUE), type="l", col=1:2)

which would mean to use lines, and to alternate colours from black to red. Or, the user might do

plot(a, which="TS", TSControl=list(separateCycles=TRUE), type="l", pch=1:2)

to alternate between two symbol types. Same with cex, lwd, lty and whatever else I might be forgetting.

Hm, I guess my examples above ought to have type and col and pch inside TSControl.

Anyway, I'll return to this tomorrow. Nothing is pushed to GH yet. I also want to re-indent the plot code, which will make a huge git diff but of course not a huge git diff --ignore-all-space, which is a default we perhaps all should be using.

[dankelley]

I've made a trial code (commit cbe399feb6177aac5bc3528745ed14902156b992 of 591_plot_TS_control branch). Sample code and the results are below. I have changed the name of the argument but I don't think explaining the scheme here is helpful -- I prefer to let the documentation provide that explanation, so that if something is unclear, it can be fixed in the place that matters.

My guess is that I've forgotten some things, and also that some old things might be broken now. That's why this is not in the develop branch yet ... I'd prefer that @richardsc weigh in first.

PS. I have extracted the relevant code to a new file, R/plot_TS.R, which ought to help in fixing bugs or adding things. There is quite a lot of commented-out test/old code there, and in R/plot.R, but my usual method is to wait until things seem settle down before removing test/old code.

library(oce)
library(argoFloats)
if (!exists("a")) # cache for debugging speed
    a <- getIndex() |> subset(ID="6990512") |> getProfiles() |> readProfiles() |> applyQC()
n <- a[["length"]]
if (!interactive())
    png("argo.png", width=7, height=7, unit="in", res=200, pointsize=10)
par(mfrow=c(2, 2))
plot(a, which="TS", TSControl=list(groupByCycle=FALSE), cex=0.5, pch=20, type="p", debug=3)
mtext("(a)")
plot(a, which="TS", TSControl=list(groupByCycle=FALSE), col=1:2, cex=0.5, pch=20, type="l", debug=3)
mtext("(b)")
plot(a, which="TS", TSControl=list(groupByCycle=TRUE), col=1:2, cex=0.5, pch=20, type="p", debug=3)
mtext("(c)")
plot(a, which="TS", TSControl=list(groupByCycle=TRUE), type="l", col=1:2, lwd=2, debug=3)
mtext("(d)")

[github-actions[bot]]

The Stale-bot has marked this issue as Stale, because no new comments have been added in the past 30 days. Unless a comment is added within the next 7 days, the Stale-bot will close the issue. The purpose of these automated actions is to prevent the developers from forgetting about unattended tasks. Note that adding a "pinned" label will turn this action off for a given issue.

[richardsc]

I just pulled and checked this, and it seems like it works as expected and is a nice addition. Sorry for being so slow on this! It was my issue to open, but @dankelley made the branch with the fix, so I'll open a PR to propose merging and will close this issue after that is done.

[github-actions[bot]]

The Stale-bot has marked this issue as Stale, because no new comments have been added in the past 30 days. Unless a comment is added within the next 7 days, the Stale-bot will close the issue. The purpose of these automated actions is to prevent the developers from forgetting about unattended tasks. Note that adding a "pinned" label will turn this action off for a given issue.

[richardsc]

596 merged, so closing this.