Addressing the reviews for JOSS submission

[JanJereczek]

Hi @Datseris!

This PR addresses most comments made by the reviewers for the JOSS publication. In particular:

• The .csv data used for Figs. 1 and 2 can now be generated from ewstools by running paper/code/ewstools-tuto-1.py. This addresses Cannot reproduce benchmarks ewstools_tuto-1.csv missing #81.
• The runtime benchmarking now relies on @belapsed and the python code has been adapted accordingly (extract min instead of mean runtime). This addresses Benchmarks in the paper should be done using benchmarking packages #80.
• The first example shown in the paper had a flaw (incorrect: tail = :right, corrected: tail = (:right, right)). This addresses Code example in JOSS paper does not work #79.
• The novelty of TransitionsInTimeseries.jl regarding extensibility is already introduced in the statement of need in paper.md.

This only leaves two points open:

• Including Mann-Kendall correlation. I think we should just open an issue for it and implement it soon but it is not required to be done for the publication.
• Passing an OffsetArray does not work with equispaced_step, as mentioned in definition of equispaced_step for OffsetArray errors #78. However, I think this is not a problem, since we want to have normal indexing, even for timeseries that have a variable dt. Your opinion is welcome!

Additionally, this PR permutes the left and right axes used in the plotting function. Looking at it again after a while, it appeared much more readable to me in this new version.

[Datseris]

Yes, open an issue for Mann-Kendall correlation. No, it is not mandatory to pass the review.

FOr the offset-arrays: I don't understand your argument. What do you mean we want to have "normal" indexing? The indexing of an array has nothing to do with its values. You can have non-equispaced data in an OffsetArray. The time vector would be such an array.

It is very easy to solve the issue. We just change the code in this file https://github.com/JuliaDynamics/TransitionsInTimeseries.jl/blob/main/src/misc/timeseries.jl to instead of use 1 to use fi = firstindex(t). And instead of use 2, 3 to use fi + 1, fi + 2. We should just fix it, it's not much code change.

[Datseris]

The .csv data used for Figs. 1 and 2 can now be generated from ewstools by running paper/code/ewstools-tuto-1.py. This addresses

Do we cite the version of ewstools? Only then the csv file can be re-generated.

[JanJereczek]

The functions of src/misc/timeseries.jl now support OffsetArrays.

The version of ewstools used for benchmarking is now specified in paper.md and a corresponding check is performed in paper/code/ewstools-tuto-1.py.

Finally, I noticed our test environment had unused (heavy) packages, which I removed.

[Datseris]

great, thanks! Would you mind writing a reply message in the JOSS review GitHub issue?

[JanJereczek]

Yes, I just left work so let me do this tomorrow!

…

On Wed, 22 May 2024, 17:59 George Datseris, ***@***.***> wrote: great, thanks! Would you mind writing a reply message in the JOSS review GitHub issue? — Reply to this email directly, view it on GitHub <#83 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJ3TCWT3AKEWU5KCFV5JE6DZDS6HXAVCNFSM6AAAAABIBWFBWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRVGE2TEMZQGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>