[JOSS] Examples in README not working

[juliohm]

Please double check the examples in the README, they are not working. The name GaussianSamplePath1D is not exported by the package for example.

[kirui93]

Hello @juliohm It is exported by the package. I can get all the exports here.

[juliohm]

This example does not work for me on a fresh Julia session:

using ScenTrees
gstree = TreeApproximation!(Tree([1,2,2,2],1),GaussianSamplePath1D,100000,2,2);
treeplot(gstree)

Are you on the latest stable version or the master branch?

[kirui93]

How did you install ScenTrees? Did you clone?

[juliohm]

No, I added it like I explained in the README PR. Using the package manager and simply ]add ScenTrees. Maybe you should release a new tag of the package so that the README is up to date with the code you expect to work.

[kirui93]

Yes. I think that is the issue. I preferred having it to be cloned because sometimes there may be additions and changes made.

[juliohm]

End users shouldn't be cloning repos because this is not stable. I suggest tagging a version with the latest working snapshot of the package, and then we can continue the review. This tag will also be used by JOSS and Zenodo in the paper for example.

[juliohm]

Furthermore, your Project.toml is outdated, it requires and old version of DataFrames.jl for example that inhibits me from installing it. Consider reviewing the Project.toml dependencies before submitting a new tag on the General registry.

[kirui93]

So what you mean is that I should make a release of the current stage of the package?

[juliohm]

You should probably revise the Project.toml dependencies, bump up the versions when you can, consider the other issues that have been raised, and run the tests before submitting a new release for publication. Makes sense?

[kirui93]

Yes, I understand. Let me do so now.

[kirui93]

Hello @juliohm I have revised the dependencies and everything is working on well. TagBot has already made a release of the new version.

[juliohm]

I tried updating but the package was not updated. Could you please confirm you used JuliaRegistrator? I am still with ScenTrees.jl v0.1.1 after ] up

[kirui93]

This is the request on Julia Registries Update

[kirui93]

The previous version, I assumed you should be working with is v0.1.2. The current version is v0.1.3.

[juliohm]

Interesting, some other package in my environment must be holding me from updating. I will try to force the installation locally in a fresh environment here.

[juliohm]

Ok, got the latest stable version in a fresh environment. Now the following example does not work:

julia> using ScenTrees
julia> gsdata = Array{Float64}(undef,1000,4)
julia> for i = 1:1000
           gsdata[i,:] = GaussianSamplePath1D()
       end
julia> gsGen = KernelScenarios(gsdata,Logistic; Markovian = true)()
4-element Array{Float64,1}:
 6.3183e-16
-1.8681
-3.7719
-3.5241

Logistic is not defined. Could you please double check?

[kirui93]

So this Logistic is the Logistic distribution in Distributions.jl package and this package is already one of the dependencies of this package. I don't understand why it doesn't pick it up. Could you please also run this julia> using Distributions before working on the example?

[juliohm]

You have two options here:

1. Update the README and all examples to add the using Distributions line.
2. Use the Reexport.jl package in your ScenTrees.jl file and add a line @reexport Distributions.

The first option is quicker, but if you think that your package does not make sense without loading Distributions.jl, then the second option could be considered in the future. Please let me know which option you prefer implementing now.

[kirui93]

I prefer the first. I am going to go with that at the moment then I will use the second later.

[juliohm]

Please let me know when the examples in the README and the docs are working as expected with the latest stable release and I will come back for the review. 👍

[kirui93]

@juliohm Everything is working perfect on the README with the current stable release. It was only the last issue you noted but I have updated on the README

[juliohm]

Thanks @kirui93 , please make sure the docs are also updated. Will come back soon with more comments if needed.

[kirui93]

Thanks for the useful comments you have made @juliohm.

• I will implement the Plots and Distributions (using Reexport.jl) issues later. Thank you so much.

[juliohm]

Thank you for improving the package. This issue is fixed. 👍