Tag?

[tpoisot]

When #39 is solved, do we want to tag the first release?

[mkborregaard]

We do I think! :tada: only wondering if we should go just a little deeper on the tests and make sure all the methods are in the docs gallery and all docstrings are defined? 🤔 And adding the page to ecojulia.org, and maybe also hosting it on ecojulia.org/NeutralLandscapes.jl ? These are the kinds of small maintenance things that tend to get left a little behind unless we self-require them before the tag :-)

[rafaqz]

The tests could do with a little work. The examples could all be doctests to make sure they actually run (not necessarily testing the output). I see they're already @example - but does an error in an @example cause CI to fail? I seem to remember that not happening for me in the past, but I'm not sure.

Also setting checkdocs=:all and strict=true in makedocs can be a good idea to make sure the docs aren't broken. Maybe that will catch errors in @example as well. But after that yeah!

[rafaqz]

We have a lot of unexported methods documented, but not included in the docs - and a few things in the docs that aren't found. The unexported docstrings could just be code comments?

┌ Warning: undefined binding '_landscape!' in `@docs` block in src/index.md:22-26
│ ```@docs
│ rand!
│ rand
│ _landscape!
│ ```
└ @ Documenter.Expanders ~/.julia/packages/Documenter/bFHi4/src/Expanders.jl:298
┌ Warning: undefined binding 'mask!' in `@docs` block in src/index.md:30-32
│ ```@docs
│ mask!
│ ```
└ @ Documenter.Expanders ~/.julia/packages/Documenter/bFHi4/src/Expanders.jl:298
[ Info: CrossReferences: building cross-references.
[ Info: CheckDocument: running document checks.
┌ Error: 25 docstrings not included in the manual:
│
│     NeutralLandscapes.NearestNeighborCluster
│     NeutralLandscapes._landscape! :: Union{Tuple{IT}, Tuple{Any,Union{DiamondSquare, MidpointDisplacement}}} where IT<:Integer
│     NeutralLandscapes._edgeMidpointCoordinates :: Tuple{AbstractArray{Tuple{Int64,Int64},1}}
│     NeutralLandscapes._rescale! :: Tuple{Any}
│     NeutralLandscapes.DiamondSquare :: Tuple{}
│     NeutralLandscapes.DiamondSquare
│     NeutralLandscapes.MidpointDisplacement
│     NeutralLandscapes._isPowerOfTwo :: Union{Tuple{IT}, Tuple{IT}} where IT<:Integer
│     NeutralLandscapes.blend :: Union{Tuple{Any,AbstractArray{T,1} where T}, Tuple{Any,AbstractArray{T,1} where T,AbstractArray{var"#s61",1} where var"#s61"<:Number}}
│     NeutralLandscapes.blend :: Union{Tuple{Any}, Tuple{Any,AbstractArray{var"#s61",1} where var"#s61"<:Number}}
│     NeutralLandscapes._subsquareCornerCoordinates :: Tuple{Int64,Int64,Int64}
│     NeutralLandscapes.label :: Union{Tuple{Any}, Tuple{Any,Any}}
│     NeutralLandscapes.classify! :: Union{Tuple{Any,Any}, Tuple{Any,Any,Any}}
│     NeutralLandscapes._initializeDiamondSquare! :: Tuple{Any,Any}
│     NeutralLandscapes._diamond! :: Tuple{Any,Any,Int64,AbstractArray{Tuple{Int64,Int64},1}}
│     NeutralLandscapes._centerCoordinate :: Tuple{AbstractArray{Tuple{Int64,Int64},1}}
│     NeutralLandscapes._ycoord :: Tuple{Tuple{Int64,Int64}}
│     NeutralLandscapes._square! :: Tuple{Any,MidpointDisplacement,Int64,AbstractArray{Tuple{Int64,Int64},1}}
│     NeutralLandscapes._square! :: Tuple{Any,DiamondSquare,Int64,AbstractArray{Tuple{Int64,Int64},1}}
│     NeutralLandscapes._diamondsquare! :: Tuple{Any,Any}
│     NeutralLandscapes.mask! :: Tuple{AbstractArray{var"#s15",N} where N where var"#s15"<:Float64,AbstractArray{var"#s16",N} where N where var"#s16"<:Bool}
│     NeutralLandscapes.classify :: Union{Tuple{Any,Any}, Tuple{Any,Any,Any}}
│     NeutralLandscapes._xcoord :: Tuple{Tuple{Int64,Int64}}
│     NeutralLandscapes._displace :: Tuple{Float64,Int64}
│     NeutralLandscapes._interpolate :: Tuple{Any,AbstractArray{Tuple{Int64,Int64},1}}

[mkborregaard]

agreed!

[rafaqz]

Ok PR on the way

[rafaqz]

Also our constructors aren't documented or consistent. Do we actually want keyword args with defaults for everything?

[mkborregaard]

yeah e.g. we need mask to be a keyword arg throughout. But you're talking the algorithm constructors? Honestly I don't really think we need default values at all but don't care much.

[mkborregaard]

There's also a bit of maybe an inconsistency between distancegradient and nearestneighborelement, where the first takes a vector of sources whereas the second takes an Int and generates that many sources. But maybe that's OK...

oh I guess we also need some badges on the readme

[mkborregaard]

I actually also think we may want to replace

function demolandscape(alg::T) where {T <: NeutralLandscapeMaker}
    heatmap(rand(alg, (200, 200)), frame=:none, aspectratio=1, c=:davos)
end

demolandscape(alg())

with

default(frame=:none, aspectratio=1, c=:davos)
gridsize = (200, 200)

heatmap(rand(alg(), gridsize))

Which gives a lot more flexibility, and also leads to example code closer to what a user would be writing

[mkborregaard]

We also need the blend functions and the nearestneighborcluster in the gallery. Wow I should really make a checklist

[mkborregaard]

When we do tag, should we just go right ahead and tag version 1.0?

[rafaqz]

I think maybe give a few minor versions to correct the mistakes weve probably made in the interface!

Currently every alg has defaults, but they aren't all documented and they are for args not kwargs so it's all or nothing.

Personally I like to use keywords, especially in demonstrating things for a paper as it's more explicit, and it's a little more flexible and loses nothing on the defualt constructors we have - so I've moved all the defaults to keywords and standardised the docs.

[rafaqz]

And we aren't actually exporting mask! now?

[rafaqz]

Yeah probably need a checklist!!

[mkborregaard]

And we aren't actually exporting mask! now?

We're not - and it would conflict with SimpleSDMLayers. But maybe we could document it and keep it as an undocumented convenience function?

[rafaqz]

Ok sure I'll document it with NeutralLandscapes.mask!

And probably we shouldn't export rand! or rand? rand is default, and rand! is using Random ?

[rafaqz]

Also should MidpointDisplacement be in its own file? it breaks the pattern nope I get it it's basically the same

[tpoisot]

When we do tag, should we just go right ahead and tag version 1.0?

I like to think of 1.0 as "production ready", so maybe we want to tag 0.1, and still allow some flexibility in case we want to adapt the API a bit?