[GRASS-dev] Added i.landsat8.swlst in grass-addons

[Nikos Alexandris]

* Blumentrath, Stefan <Stefan.Blumentrath at nina.no> [2016-05-06 13:09:31 +0000]:

>Dear Nikos,
>
>I just tested your i.landsat8.swlst module. Cool stuff!

Thank you (so) very much for testing!


>During my experiments I ran into one issue and identified some (minor)
>room for speed-ups (mainly replacing g.copy with g.rename).

Nice :-)


> I did not
>do a full code review but made some suggestions in a pull request on
>github. I tested all changes and they seem to work fine.

I tested your changes, it works. Merged.  Some questions I (might) have (for my own
learning though), I'll put as comments in github.


>Some other things I noticed beyond my changes:
>
>-        For me the manual did not build locally (meaning it was not available in the GUI)

It builds for me, not error-free though (missing images).


>-        Given the fact that the FROM-GLC data has not been updated for
>a while (if I did not get the link wrong), I would encourage users to
>provide their own land cover map, possibly reclassified from
>topographic maps. The only one available at
>http://data.ess.tsinghua.edu.cn/ for my scene was not only a couple of
>years old, but also quite clowdy in addition.

Yes.  Have you already done this for your area of interest?


>-        The k-flag (keep current computational region) is invers to GRASS standards. Maybe better to switch the behavior?

Not sure.  It's kind of related to the #1 mistake everyone does, forget
setting the right region extent.  So, I'd thought of letting the module
operate on the whole scene.

Do we have a reference for this ("GRASS standards") or you
mean the "common" behaviour of most modules?

But, in any case, I am all open to any "good" change(s)!  If you have changed this
locally, please include it in a pull request.


>-        The GUI would – for my taste – benefit from splitting the
>options over different tabs (guisection). Now most options and flags
>end up in “optional” which makes it a bit confusing for newcomers to
>see what is input and what output...

Absolutely!  This is simply work not done.  I tried hard, really, to be clean
with the flags.  I have to re-read the script, I almost don't remember a
thing at the moment!  I think I let it be like that because I did not
resolve on what and how should be required, and how this would affect
other options.  Need to re-read.

Thank you Stefan, really awesome you took the time to read and apply
corrections.  It's most rewarding to learn collaboratively!

Kindest regards, Nikos