E3SM-Project / ChemDyg

Chemistry Diagnostics Package
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

Consider separating python and bash files #5

Open xylar opened 1 year ago

xylar commented 1 year ago

It seems that ChemDyg followed the example of: https://github.com/E3SM-Project/zppy/blob/c3f70dc8c814c36dcad5c380b1f86c8475a6192e/zppy/templates/e3sm_diags.bash in creating python scripts within bash scripts.

This is not a very good software practice, since it's pretty confusing to debug and contribute to. Instead, zppy has a better example to work from: https://github.com/E3SM-Project/zppy/blob/c3f70dc8c814c36dcad5c380b1f86c8475a6192e/zppy/templates/global_time_series.bash https://github.com/E3SM-Project/zppy/blob/c3f70dc8c814c36dcad5c380b1f86c8475a6192e/zppy/templates/coupled_global.py

The bash script calls the python script: https://github.com/E3SM-Project/zppy/blob/c3f70dc8c814c36dcad5c380b1f86c8475a6192e/zppy/templates/global_time_series.bash#L90 with several arguments. These arguments are then taken from the parameters array and copied into python variables: https://github.com/E3SM-Project/zppy/blob/c3f70dc8c814c36dcad5c380b1f86c8475a6192e/zppy/templates/coupled_global.py#L491-L502

An even better practice would be to use the argparse package: https://docs.python.org/3/library/argparse.html Here is an example from e3sm_diags of how that can be used: https://github.com/E3SM-Project/e3sm_diags/blob/833d9b2df0515565bf4150b20101e80dea647294/auxiliary_tools/metrics_checker.py#L21-L42

Once #1 is taken care of, I would be happy to provide an example for one of the existing .bash templates that you all could use to make similar changes to the others.