CABLE-LSM / benchcab

Tool for evaluation of CABLE land surface model
https://benchcab.readthedocs.io/en/latest/
Apache License 2.0
2 stars 4 forks source link

#140 - Alternate config_options style #137

Closed ccarouge closed 1 year ago

ccarouge commented 1 year ago

Modifications to config_options.md in the documentation to improve readability:

codecov[bot] commented 1 year ago

Codecov Report

Merging #137 (efc9b36) into master (44873fc) will increase coverage by 0.29%. Report is 8 commits behind head on master. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #137      +/-   ##
==========================================
+ Coverage   88.47%   88.77%   +0.29%     
==========================================
  Files          27       27              
  Lines        1527     1630     +103     
==========================================
+ Hits         1351     1447      +96     
- Misses        176      183       +7     
Files Changed Coverage Δ
benchcab/internal.py 90.47% <ø> (+0.23%) :arrow_up:

... and 9 files with indirect coverage changes

SeanBryan51 commented 1 year ago

Nice. Can you replace all the options that are headings to

[`option`](#+heading.option){ #+heading.option }

I think it would be best if all the options had a consistent look.

ccarouge commented 1 year ago

Absolutely. This was just a first try to see what it would look like and if it would improve things before going all the way.

ccarouge commented 1 year ago

Actually, I don't think I want all the options to be links instead of headings. I think we want to keep headings for some of them so they appear in the table of contents. My first try is with only the innermost options as "links" and the other levels of options are headings. I was somewhat taking inspiration from here. It uses headings for all keys that have no values and uses the "link" approach for the sub-keys with values.

Obviously, we have keys at the top level that have values so I'm not sure what will be the best for that case.

ccarouge commented 1 year ago

@SeanBryan51 I'm curious to see what you think of it now.

SeanBryan51 commented 1 year ago

Looks good to me!

ccarouge commented 1 year ago

@bschroeter Sean and I have seen this documentation way too many times. I think some fresher eyes for the review would be good.

ccarouge commented 1 year ago

@bschroeter you can see a preview of the docs, by clicking on "Details" for the docs/readthedocs.org:benchcab test.

ccarouge commented 1 year ago

@bschroeter

The admonition used to alert the user to all items being required at the top of the page could be the exclamation point (!!! warning).

Actually, I think we can completely remove this. I have now specified for each option whether it is optional or required. So that the look is consistent between options. But I've realised I've used required option but optional key. I'll changed all to key.

ccarouge commented 1 year ago

@bschroeter About the "put it all together", we already tell users to start from the bench_example repository. This already contains an example yaml file with all the required options (but not the optional ones). Is that enough? Do you think we should link to it on this doc page? I don't really want to have a copy as they are going to get out of sync.

If people come from the User Guide to this page to see the options, it is linked from this paragraph:

Once the work directory is cloned, you will need to adapt the config.yaml file to your case. Refer to the description of the options for this file.

So they should have the example open on their screen.

bschroeter commented 1 year ago

@bschroeter About the "put it all together", we already tell users to start from the bench_example repository. This already contains an example yaml file with all the required options (but not the optional ones). Is that enough? Do you think we should link to it on this doc page? I don't really want to have a copy as they are going to get out of sync.

If people come from the User Guide to this page to see the options, it is linked from this paragraph:

Once the work directory is cloned, you will need to adapt the config.yaml file to your case. Refer to the description of the options for this file.

So they should have the example open on their screen.

This is a fair assumption. I can appreciate that the example on the docs will likely get out of date with what is installed.