Conversation
LSchueler
left a comment
There was a problem hiding this comment.
Hi Victor,
I am truly sorry for keeping you waiting so long for a response! I like the idea of supporting the madogram. But before we can merge your PR, there are a few small things I would like to see changed.
The new functionality is not mentioned anywhere in the docstrings. Could you add some text to the docstring of the vario_estimate function in the file variogram.py? And could you also add a few simple unit tests?
Apart from those two points and the very small comments I made inline, I think we are nearly good to go. Thanks again!
Cheers,
Lennart
based on commit resolve discussion. estimator changed to `a`.
included relevant citation.
|
Thank you for the changes. You should include your references there too and then you can link them via |
added estimator F - madogram