EcoJulia / NeutralLandscapes.jl

Generation of neutral landscapes in Julia.
https://ecojulia.github.io/NeutralLandscapes.jl/dev/
MIT License
16 stars 2 forks source link

refeactor and docs #49

Closed rafaqz closed 3 years ago

rafaqz commented 3 years ago

What the pull request does
Refactors and cleans up a little, standardises all algorithm arguments and docs, and fixes documentation so there are no warnings - which means documenting everything exported, and moving a lot of docs to code comments.

Type of change
Please indicate the relevant option(s)

Checklist

codecov-io commented 3 years ago

Codecov Report

Merging #49 (cecab92) into main (18df1db) will decrease coverage by 0.74%. The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
- Coverage   62.45%   61.71%   -0.75%     
==========================================
  Files          14       14              
  Lines         277      269       -8     
==========================================
- Hits          173      166       -7     
+ Misses        104      103       -1     
Flag Coverage Δ
unittests 61.71% <50.00%> (-0.75%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/NeutralLandscapes.jl 100.00% <ø> (ø)
src/algorithms/discretevoronoi.jl 0.00% <0.00%> (ø)
src/algorithms/distancegradient.jl 0.00% <ø> (ø)
src/algorithms/edgegradient.jl 100.00% <ø> (ø)
src/algorithms/nncluster.jl 0.00% <0.00%> (ø)
src/algorithms/nnelement.jl 94.44% <ø> (-0.30%) :arrow_down:
src/algorithms/nogradient.jl 100.00% <ø> (ø)
src/algorithms/perlinnoise.jl 89.65% <ø> (ø)
src/algorithms/planargradient.jl 100.00% <ø> (ø)
src/algorithms/rectangularcluster.jl 100.00% <ø> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 18df1db...cecab92. Read the comment docs.

mkborregaard commented 3 years ago

But why do we want docstrings on unexported functions that are never meant to be user-facing?

tpoisot commented 3 years ago

But why do we want docstrings on unexported functions that are never meant to be user-facing?

Maybe that's just me, but I like having the docstring when I'm writing things for the package - as in, when I hover the function name in VS Code - it's easier than having to look into the code for the comments.

mkborregaard commented 3 years ago

It seems a little strange to me, but I don't feel strongly about it, as long as they are kept out of the docs :-)

rafaqz commented 3 years ago

If you have docstrings not in the docs, you get a warning so tests wont pass when strict=true. If you turn off strict to fix this, they will pass when a new PR misses a docstring or messes something up.

The idea was to enforce documentation. Its made my life a lot easier on my own packages to get test failures whenever the docs break in any way, and made the docs more reliable.

Otherwise Im easy either way. But underscore functions are for devs only, so comments seem right to me anyway?

And why not use @kwdef? I guess we can write the constructors out. But it is a base macro.

tpoisot commented 3 years ago

Let's use @kwdef if it makes it user frieldier. I still think internal functions should be documented - a thing that many packages do is to have a specific page for them called "internals" or something. It can start with a disclaimer that people should probably not use these functions for daily use.

mkborregaard commented 3 years ago

I'm OK with having them under "internals" if that is useful for you guys. And I don't care either way about kwdef :-) Then we just have some docstrings to write - I didn't docstring any internal functions.

tpoisot commented 3 years ago

I'm fine with the package as is, so I'm going to merge and tag as soon as it passes. I'll start building the SimpleSDMLayers integration as soon as we have a version!