logan-berner / LandsatTS

LandsatTS is an R package to facilitate retrieval, cleaning, cross-calibration, and phenological modeling of Landsat time-series data.
Other
26 stars 4 forks source link

Documentation #1

Closed jakobjassmann closed 2 years ago

jakobjassmann commented 3 years ago

Jakob:

Logan:

jakobjassmann commented 3 years ago

Logan: Once you have a chance, it would be great if you could:

  1. Go over the documentation and expand your sections in the readme.
  2. Provide fully self-contained, meaningful and run-able examples in the roxygen documentation for each function. For now all your examples failed the checks and instead of including them, I had to comment them all out. Have a look at the two functions that I wrote and the tests for them to see how examples / tests could be generated. Get in touch if you have any questions!
logan-berner commented 3 years ago

Yesterday I expanded the readme section. Still need to develop fully self-contained examples...

scelmendorf commented 3 years ago

On the documentation thread, I am a little confused about what lsat_calibrate_rf is returning in the resulting data table. I think (?) there is a new column called [band].cal (e.g. ndvi.xcal) which I assumed was the 'corrected' ndvi that one would want to used in the downstream analyses. But then with this interpretation I think one would want ndvi.xcal to be used in the next steps in the readme/vignette, but it's still ndvi. So I think I am missing something on the documentation end that would clarify this, perhaps others as well?

logan-berner commented 3 years ago

Good eye @scelmendorf ! Thanks for pointing that out. Yes, the appended [band].xcal data are the corrected ndvi. As you noted, you'll either want to use ndvi.xcal in the subsequent functions, or replace the uncalibrated data with the calibrated data under the ndvi column. I updated the documentation to make that workflow clearer.

scelmendorf commented 3 years ago

More documentation: I don't fully understand the consequences of assigning points or polygons to different regions. I think (?) the RF cross-validation works by site(?). And I understand how we have often used the region concept for synthesis papers. But analytically, I don't know what it does here.

jakobjassmann commented 3 years ago

@scelmendorf This is a really helpful comment! There is no meaning behind assigning points or polygons to regions. When I read your comment I was surprised that you got that impression, but I re-read the readme and I think that the second example for lsat_get_pixel_centers() is perhaps confusing to the reader.

In the example the identifier "region" is used to provide a unique identifier to the different polygons, which can then be used to split the multi row polygon sf object into unique polygons to map lsat_get_pixel_centers() over each of them. The column name "region" carries no meaning at all. I just chose "region" instead of "site", as Logan requested that we reserve "site" for the individual point coordinates. Perhaps calling the column "polygon_id" in the example would be less confusing?

Very happy to hear any other suggestions to improve clarity on this end! :)

scelmendorf commented 3 years ago

more spelling minutia: "parameter" is misspelled in documentation for lsat_fit_phenological_curves spar
Smoothing paramater passed to smooth.spline(), typically around 0.65 - 0.75 for this application.

logan-berner commented 3 years ago

Thanks @scelmendorf! Fixed the "parameter" typo.

jakobjassmann commented 2 years ago

Continued in #28 .