Feedback: User Documentation

goergen95 commented 2 years ago

Hi @Jo-Schie and @Ohm-Np, I updated the package to include some user-facing documentation. It would be great if you could read through it and we can collect any potential feedback here in this issue. You can follow this link to get to the online version and click through the pages in the articles section of the navbar.

Jo-Schie commented 2 years ago

Hi @goergen95 can you copy quickly the link to the Ressource so i can edit it directly

goergen95 commented 2 years ago

Sorry, what resource do you mean?

goergen95 commented 2 years ago

The vignettes are found here.

Jo-Schie commented 2 years ago

this is in main correct? So id have to push changes to the introduction to main?

goergen95 commented 2 years ago

Yes, no vignettes in other branches yet. If you want you can create a branch for the documentation review and open a PR.

Jo-Schie commented 2 years ago

Here is one suggestion that we noted just yesterday when going through the documentation: We were unable to find an overview of all resources and indicators. That should be prominently linked. Maybe it could be even a single page in the vignette or something.

goergen95 commented 2 years ago

please find an updated version of the userguide (now referred to as "quickstart") online. I would suggest that we include an overview of all supported resources and indicators in the README. That yet has to be properly desgined anyway.. From there we could directly link to the quickstart vignette. What do you think?

melvinhlwong commented 2 years ago

Thank you for your work. Great package, great documentation. It is very detailed and a great help for new users. I have committed couple of changes to the branch "edits_manual". They are mostly typo correction and style changes to shorten sentences.

Here are some comments in addition to my commits.

Readme file

I added some changes in another branch. I will make a pull request at the end of my revision

[x] "This package is platform agnostic and provides the possibility to e.g. analyze the World Database of Protected Areas (WDPA)." It appears to be two sentences. Also, what does agnostic mean? Add short sentence?

I removed the sentence. Also I redesigned the readme, giving a very quick overview of what the package actually does. Would be great to receive your feedback on that.

Introduction.Rmd

[x] "rectangular structure". What is meant with that?

reformulated to: "the results of the indicator calculation will be added to the portfolio object as nested list columns. This approach makes it feasible to support a variety of indicators with differently shaped outputs (e.g. time variant vs. invariant indicators)."

workflow.Rmd

[x] What is meant with "a standardized interface". It is not defined beforehand
[x] The bullet points appear to duplicate the text in the paragraph. Consider deleting the text and keep only small intro sentence, bullet points, and figure

followed your recommendation and removed the text. Also I inserted a new bullet point that says "identify the required resources needed for the indicators you want to calculate". Also the term "standardized interface is no longer used.

quickstart.Rmd

[x] The paragraph starting with "The add_resources arguments controls..." is unclear to me. Could you rewrite? After reading this, I do not understand what it does and what the implications are.

_reformulated to: "In case your share a common outdir across different portfolios, with the add_resources arguments controls the behavior of the portfolio initialization. If set to TRUE, the function will automatically search for all available resources and add these as attributes to the portfolio, without further checking if these resources actually match its spatio-temporal extent. Only use this if you are sure that for your current portfolio, all resources have been previously downloaded. If set to FALSE, no pre-existing resources will be added to the portfolio. Once you request a specific portfolio for your resources, only those files will be downloaded that are missing in outdir. This behavior is beneficial if you share the outdir between different projects to ensure that only matching resources are returned."_

[x] "verbosity" could you describe what it does without using the word?

reformulated to: "Finally, the verbose logical controls whether or not the package will print informative messages during the calculations."

[x] "sample_portfolio <- get_resources(x = sample_portfolio, resources = "chirps")" Here you get the resources for one dataset. How can one get multiple resources? Via resources = c("chirps", "other_resource")? is it possible to add this as text somewhere? Maybe it could be added in here as "do not run" example? https://mapme-initiative.github.io/mapme.biodiversity/reference/get_resources.html

added an example with two resources and an additional argument:

sample_portfolio <- get_resources(x = sample_portfolio, 
                                  resources = c("chirps", "treecover2000"),
                                  vers_treecover = "GFC-2020-v1.8")

[x] default values: The code output says "Argument 'scales_spi' for resource 'chirpsprec' was not specified. Setting to default value of ''". "SPI has to have a time scale defined and it is not listed in the output nor is the default value listed in the indicator help file. Could you do two things? 1) Make sure that the default values are indicated in the code output? 2) set the SPI default scale to 3 months. I think this is a useful default for the SPI in terms of time scales.

The default value for SPI was set to NULL, meaning if users request the chirpsprec indicator without specyfing anything not SPI calculation would be done. Printing this default NULL value lead to the empty string output you mentioned above. I changed the default value to 3, per your suggestion, and also now explicitly specify all user-required arguments.

sample_portfolio <- calc_indicators(sample_portfolio,
  indicators = "chirpsprec",
  scales_spi = 3,
  spi_prev_years = 8,
  engine = "extract"
)

[x] The example of the SPI is confusing. I was not sure which indicators you are about to create. Precipitation or SPI? In fact you calcualate both. Could you add a specification what exact indicators you are calculating? Maybe you can add a sentence in the section "Calculate specific indicators".

actually, in the example as it was, SPI was not calculated (see comment above) but we only calculated absolute precipiation and precipitation anomaly. Following your suggestion, now the SPI is also explicitly calculated. I hope that makes the example less ambiguous.

[x] Graph of SPI anomaly. The graph does not show which time scale has been used to the SPI. Could you change that, so it appears in the graph? I do not think we need to specify the defaults in the graphs generally. However, one problem with the SPI or SPEI is that people often do not know what the time scales mean and assume that there is this one indicator for anomalies (e.g. drought). The time scale are an important concept and having this in the label is not only correct, but triggers people to question what it means

see comment above. The second graph was actually the anomaly, not SPI, which does not have a time-scale in that sense (actually it does because anomaly is calculated with reference to the 1981-2010 climate normal period). With the new example a third plot is added for SPI and its time scale is made explicit.

[x] "However, users should note that the attributes that were added during the portfolio initialization are not reconstructed." Not sure what that means. Can you add a sentence after this sentence to explain? E.g. "... portfolio initialization are not reconstructed. That is, ..."

reformulated to: "However, users should note that the portfolio-wide arguments that were set during the portfolio initialization are not reconstructed (e.g. the temporal extent, outdir and tmpdir, etc.)."

drought_indicator.html

[x] What is the unit of the indicators? It says wetness, but not which unit? Is it cm of underground water thickness?

_That indicator was unlcear to me too. I added the following to the description of the nasagrace resource: "The resource is published by NASA GRACE Tellus. This data set reflects on potential drought conditions in the shallow groundwater section relative to a reference period spanning from 1948 to 2012. It is available as a global raster with a weekly temporal resolution starting with the year 2003. The value indicates the wetness percentile of a given pixel with regard to the reference period." and to the drough_indicator documentation: "#This function allows to efficiently calculate the relative wetness in the shallow groundwater section with regard to the the 1948-2012 reference period. The values represent the wetness percentile a given area achieves at a given point in time in regard to the reference period."_

[x] Could you check for each indicator that the units are stated? Many have them but accessibility.html and drought_indicator.html, for example, do not have units stated

I just found some missing units but not for accessibility, Its units were specified in the format section

goergen95 commented 2 years ago

@melvinhlwong please see my edits in your comment above.

mapme-initiative / mapme.biodiversity