Feature 918 add use case template #2690
Conversation
j-opatz
requested review from
DanielAdriaansen,
georgemccabe and
lisagoodrich
There was a problem hiding this comment.
I have a few suggestions that are up for discussion:
- Should we put this template in its own file and include it in the documentation? That way users could copy the file directly from the repo.
- Should we make the template viewable in ReadTheDocs so users can see how the content of the template is rendered?
- In the template, I see specific references to existing files, use cases, etc. Should these be renamed to be generic so it is more clear that they are from the template and should be modified or removed? I worry that it will be easy to miss some of these on review if it is not clear that they are unmodified from the template and incorrect information could make its way into other use cases.
- It looks like the template includes text to describe each section that I presume needs to be removed by the user. I suggest we either:
- Put that information in the template formatting so that it ends up being rendered content that will be replaced by the user's actual content
- Put the header and corresponding information about that section in the documentation below the template so it is not embedded within it and users can refer to it while modifying their template
Please let me know what you think and/or if my suggestions are unclear. Also, I apologize if some of these details have already been discussed while I was out.
| @@ -209,56 +209,425 @@ Add Sphinx Documentation File | |||
| In the corresponding documentation MET tool name directory | |||
| (**docs**/*use_cases/met_tool_wrapper/<MET TOOL NAME>*) for a met_tool_wrappers | |||
There was a problem hiding this comment.
docs should not be bolded but in italics
| @@ -209,56 +209,425 @@ Add Sphinx Documentation File | |||
| In the corresponding documentation MET tool name directory | |||
| (**docs**/*use_cases/met_tool_wrapper/<MET TOOL NAME>*) for a met_tool_wrappers | |||
| use case OR category directory for a model_applications use case | |||
| (**docs**/*use_cases/model_applications/<CATEGORY>*), add: | |||
| (**docs**/*use_cases/model_applications/<CATEGORY>*), users will need to a add | |||
There was a problem hiding this comment.
docs should not be bolded but in italics
| The following use case template has been provided and is expected to be completed by all users | ||
| submitting new use cases. The template applies to both met_tool_wrappers and model_applications use cases. | ||
| When completing the template, users should read through each section and its description | ||
| and example before filling in the section, as some section may not apply |
There was a problem hiding this comment.
add plural sections:
as some sections may not apply
| # a start time in YYYYMMDDHHMMSS, an end time in the same format, a message type to code | ||
| # the variables as, and a variable name to read in. Currently the script puts the | ||
| # same station ID to each observation, but there is space in the code describing | ||
| # an alternate method that may be improved upon to allow different sattellites to have |
There was a problem hiding this comment.
change "sattellites" to "satellites"
| to the use case being documented. For reference, users are encouraged to review | ||
| existing `Model Applications <https://metplus.readthedocs.io/en/latest/generated/model_applications/index.html>`_ | ||
| use case documentation. Except for the Header and Path to Use Case Configuration File section, | ||
| all lines should begin with the '#' character to signify text, followed by at least one space before |
|
|
||
| Header and Path to Use Case Configuration File | ||
|
|
||
| This section begins with {PrimaryStatAnalysisToolName}: Brief (12 words or less) and a unique |
|
|
||
| This represents the METplus version number that this use case was added to | ||
| the METplus repository. It also represents the minimum or lowest METplus version | ||
| this use case has been tested against. It should not include betas (ex. Version 5.1 beta 3), |
|
|
||
| This section should include a brief description of each model dataset | ||
| and variable field (10 m wind speed, 2 m temperature, etc.) being used | ||
| in the use case, as well as which field (forecast, observation, climatology, masking, etc.) |
| This section should include a brief description of each model dataset | ||
| and variable field (10 m wind speed, 2 m temperature, etc.) being used | ||
| in the use case, as well as which field (forecast, observation, climatology, masking, etc.) | ||
| is using which dataset. At a minimum, users should list the Forecast, Observation, and Climatology |
|
|
||
| This section lists the tools that will be used during the use case. | ||
| If there are multiple tools, a brief overview should be provided of what each tool | ||
| is responsible for (i.e. GenVxMask for creating masks that are used in the verification step, |
| This section lists the tools that will be used during the use case. | ||
| If there are multiple tools, a brief overview should be provided of what each tool | ||
| is responsible for (i.e. GenVxMask for creating masks that are used in the verification step, | ||
| which is completed by GridStat). If Python embedding is used, it can be mentioned here as well. |
| # ----------------- | ||
| # | ||
| # METplus sets environment variables based on user settings in the METplus | ||
| # configuration file. See :ref:`How METplus controls MET config file settings<metplus-control-met>` for more details. |
| # METplus sets environment variables based on user settings in the METplus | ||
| # configuration file. See :ref:`How METplus controls MET config file settings<metplus-control-met>` for more details. | ||
| # | ||
| # **YOU SHOULD NOT SET ANY OF THESE ENVIRONMENT VARIABLES YOURSELF! THEY WILL BE OVERWRITTEN BY METPLUS WHEN IT CALLS THE MET TOOLS!** |
| # | ||
| # **YOU SHOULD NOT SET ANY OF THESE ENVIRONMENT VARIABLES YOURSELF! THEY WILL BE OVERWRITTEN BY METPLUS WHEN IT CALLS THE MET TOOLS!** | ||
| # | ||
| # If there is a setting in the MET configuration file that is currently not supported by METplus you’d like to control, please refer to: |
| # .. literalinclude:: ../../../../parm/met_config/GridStatConfig_wrapped | ||
|
|
||
|
|
||
| Python Embedding |
There was a problem hiding this comment.
Please break up this section so the user doesn't have to horizontally scroll to read it
| # | ||
| # This use case calls the read_ASCAT_data.py script to read and pass to PointStat | ||
| # the user-requested variable. The script needs 5 inputs in the following order: | ||
| # a path to a directory that contains only ASCAT data of the “ascat_YYYYMMDDHHMMSS_*” string, |
| # their own station IDs. This code currently ingests all files it finds in the directory, | ||
| # pulls out the requested variable, and arranges the data in a list of lists | ||
| # following the 11-column format for point data. This list of lists is passed back | ||
| # to PointStat for evaluation and the requested statistical output. The location of the code is |
| This section should provide a description of any Python scripting that’s used | ||
| in the use case. For a common definition, Python scripting is any condition where | ||
| the evaluation/verification/output processes are being completed inside the Python script, | ||
| outside of METplus. Essentially, if a Python script is called and a METplus-readable dataset |
| is not passed back to METplus, it is a Python Scripting usage. The METplus wrapper usage only exists | ||
| to call the Python script. | ||
| This section should discuss what is being done by the script and why the decision was made | ||
| to use Python scripting rather than Python embedding. The end of this section is an embedded link (and image) |
| successful METplus run concludes with. Then, it should direct users to the | ||
| proper folder (folders, if multiple outputs are made) and directory structure | ||
| where the final output is. If the use case creates intermediate output, it should | ||
| be mentioned here as well. A list of the files and folders that are created should be provided. |
| where the final output is. If the use case creates intermediate output, it should | ||
| be mentioned here as well. A list of the files and folders that are created should be provided. | ||
| If a netCDF is the output, it should be listed how many and what variable fields | ||
| are inside the file. If there are a large number of variable fields within the file, a summary is sufficient. |
| be mentioned here as well. A list of the files and folders that are created should be provided. | ||
| If a netCDF is the output, it should be listed how many and what variable fields | ||
| are inside the file. If there are a large number of variable fields within the file, a summary is sufficient. | ||
| If an image is created, it should be used as the use case image and referenced in this section as output. |
| If a netCDF is the output, it should be listed how many and what variable fields | ||
| are inside the file. If there are a large number of variable fields within the file, a summary is sufficient. | ||
| If an image is created, it should be used as the use case image and referenced in this section as output. | ||
| If no output is created, this section should explain why and what the user accomplished by running the use case. |
| # * grid_stat_198201_000000L_19700101_000000V.stat | ||
| # | ||
| # Each file should contain corresponding statistics for the line type(s) requested. | ||
| # For the netCDF file, five variable fields are present (not including the lat/lon fields). Those variables are: |
| before using it in the use case documentation. | ||
| Users should also use the end of this section to reference an image that | ||
| will serve as a thumbnail for the use case. If no image is provided, | ||
| a default image will be used; this is the same image used for all met_tool_wrapper use cases. |
There was a problem hiding this comment.
Great work @j-opatz! This was a lot to add. Thanks for doing all the heavy lifting on this.
I think @georgemccabe brings up some good points.
My changes are more nit-picky and related to presentation and grammar. I would like to see more line breaks so the reader doesn't have to horizontally scroll to read the text.
|
@lisagoodrich thanks for your grammar and syntax checks. I've gone ahead and updated what I could for your suggestions. Some of the text is unable to be folded across multiple lines due to links or character effects, but it should be more manageable now. @georgemccabe, I wanted to reply to your suggestions/questions in line, as there was a bit more to unpack than just corrections to content.
While this wasn't discussed previously, I do like this idea more than my previous write-up. It does rely on users following links and reading documentation in two places (which is more potential for errors/skipping instructions), but it de-clutters the current documentation location, and collects all of the example text into one area which may actually encourage proper reading by users.
I think we should avoid this situation. The template is simply a template; if the user wants to see what it renders like, then they should be looking at the numerous use case documentation examples we have. Is there a way we can make sure the example template does not render in RTD?
I'm hoping by moving the example template to its own file, as well as stating in the discussion of the template what needs to be updated, we should capture most of the "whoops, forgot" moments. My worry by keeping things generic is that users won't understand what to replace it with for their specific use case. And if users are going to miss it because they're going too fast or didn't see it, the content inside it won't help regardless of generality or specificity.
I think this concern is addressed by separating the discussion and the examples. |
|
You could put the template .py file in docs/use_cases? It looks like we render the use cases chapter by referencing the docs/use_cases/met_tool_wrapper and docs/use_cases/model_applications files, so the template shouldn't be included in the User's Guide. |
…ords, testing 2 separate options to stop paragraph jumping during bullet lists
|
Many updates have been made to the template and accompanying documentation. This includes instructions for adding a new keyword. A second review would be beneficial at this point, to check for any lingering issues. Note that the template material has now been moved to docs/use_cases/use_case_documentation_template.py |
Pull Request Testing
Describe testing already performed for these changes:
Documentation generated from RTD was reviewed for accuracy when viewed online.
Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Reviewers should look at how the material appears in the online guide here: https://metplus.readthedocs.io/en/feature_918_add_use_case_template/Contributors_Guide/add_use_case.html#add-sphinx-documentation-file
This material should also be reviewed for its content, which can be done in the "Files Changed" tab.
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
Do these changes include sufficient testing updates? [Yes]
Will this PR result in changes to the test suite? [No]
If yes, describe the new output and/or changes to the existing output:
Do these changes introduce new SonarQube findings? [No]
If yes, please describe:
Please complete this pull request review by [9/20].
Pull Request Checklist
See the METplus Workflow for details.
Select: Reviewer(s) and Development issue
Select: Milestone as the version that will include these changes
Select: Coordinated METplus-X.Y Support project for bugfix releases or METplus-Wrappers-X.Y.Z Development project for official releases