From a4635530ad5f11402c2ef7fdad946c273b07958f Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Mon, 10 Jan 2022 16:19:16 -0700 Subject: [PATCH 01/10] Add a design document for command-line arguments --- doc/design/command_line_arguments.rst | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 doc/design/command_line_arguments.rst diff --git a/doc/design/command_line_arguments.rst b/doc/design/command_line_arguments.rst new file mode 100644 index 0000000000..da3ff0d815 --- /dev/null +++ b/doc/design/command_line_arguments.rst @@ -0,0 +1,24 @@ +.. sectnum:: + +.. contents:: + +================================== + Overview of this design document +================================== + +This documents our conventions for command-line arguments, primarily as they pertain to python scripts. These primarily focus on user-visible aspects, as opposed to implementation details. + +======================================== + Conventions for command-line arguments +======================================== + +1. Options that are more than a single character should be formatted as ``--some-variable`` -- i.e., with a double leading hyphen and a hyphen as word separator. + - Rationale: This follows the `POSIX conventions `_. +2. Scripts should support ``--verbose`` / ``-v`` and ``--debug`` options. See comments at the top of ``ctsm_logging.py`` for details. +3. For arguments that need a value: If there is no obvious default for the value, then the argument should be required rather than being given a default that the user will often need to override. A general rule of thumb is: if, say, 75% -- 90% of users will want a particular value, then it can be okay to have this value as the default; otherwise, it should be a required argument. This holds even for arguments that are only needed in certain circumstances: For example, if argument ``--fooval FOOVAL`` needs to be given whenever ``--bar`` is given, but there is no obvious default for ``FOOVAL``, then the argument parsing should be coded so that ``--fooval`` does not have a default, and there is error checking code that ensures that ``fooval`` has been specified (not ``None``) if ``--bar`` was given. + - Rationale: Giving an argument a default value means that we can't catch and report when a user forgets to set it but should have set it. Leaving off the default allows for this error checking. +4. For logical flags, use a flag without an argument -- ``--feature`` for the case where something is off by default and you want to turn it on, or ``--no-feature`` for the case where something is on by default and you want to turn it off -- instead of something like ``--feature true`` / ``--feature false``. Prefer phrasing these as positives whenever possible (e.g., ``--allow-multiple-pfts`` instead of ``--no-single-pft``). To the extent possible, try to clearly document what the behavior is if you add the flag, and what the behavior is if you don't add the flag. + - Rationale: + + - This use of ``--feature`` / ``--no-feature`` is more common behavior for Unix tools. + - For something that is either on or off by default, you can see what the default operation is and what you can change just by looking at the available flag names, without needing to read through all of the documentation of default values. From a9a16b32693e872b58121bc1e5fafe3bfed03c7d Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Mon, 10 Jan 2022 16:25:35 -0700 Subject: [PATCH 02/10] Try to fix formatting --- doc/design/command_line_arguments.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/doc/design/command_line_arguments.rst b/doc/design/command_line_arguments.rst index da3ff0d815..f9bcb91180 100644 --- a/doc/design/command_line_arguments.rst +++ b/doc/design/command_line_arguments.rst @@ -13,12 +13,18 @@ This documents our conventions for command-line arguments, primarily as they per ======================================== 1. Options that are more than a single character should be formatted as ``--some-variable`` -- i.e., with a double leading hyphen and a hyphen as word separator. - - Rationale: This follows the `POSIX conventions `_. + + * Rationale: This follows the `POSIX conventions `_. + 2. Scripts should support ``--verbose`` / ``-v`` and ``--debug`` options. See comments at the top of ``ctsm_logging.py`` for details. + 3. For arguments that need a value: If there is no obvious default for the value, then the argument should be required rather than being given a default that the user will often need to override. A general rule of thumb is: if, say, 75% -- 90% of users will want a particular value, then it can be okay to have this value as the default; otherwise, it should be a required argument. This holds even for arguments that are only needed in certain circumstances: For example, if argument ``--fooval FOOVAL`` needs to be given whenever ``--bar`` is given, but there is no obvious default for ``FOOVAL``, then the argument parsing should be coded so that ``--fooval`` does not have a default, and there is error checking code that ensures that ``fooval`` has been specified (not ``None``) if ``--bar`` was given. - - Rationale: Giving an argument a default value means that we can't catch and report when a user forgets to set it but should have set it. Leaving off the default allows for this error checking. + + * Rationale: Giving an argument a default value means that we can't catch and report when a user forgets to set it but should have set it. Leaving off the default allows for this error checking. + 4. For logical flags, use a flag without an argument -- ``--feature`` for the case where something is off by default and you want to turn it on, or ``--no-feature`` for the case where something is on by default and you want to turn it off -- instead of something like ``--feature true`` / ``--feature false``. Prefer phrasing these as positives whenever possible (e.g., ``--allow-multiple-pfts`` instead of ``--no-single-pft``). To the extent possible, try to clearly document what the behavior is if you add the flag, and what the behavior is if you don't add the flag. - - Rationale: - - This use of ``--feature`` / ``--no-feature`` is more common behavior for Unix tools. - - For something that is either on or off by default, you can see what the default operation is and what you can change just by looking at the available flag names, without needing to read through all of the documentation of default values. + * Rationale: + + * This use of ``--feature`` / ``--no-feature`` is more common behavior for Unix tools. + * For something that is either on or off by default, you can see what the default operation is and what you can change just by looking at the available flag names, without needing to read through all of the documentation of default values. From 62c387fd2f0e06eccd3eace572091325fc5cfb25 Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Mon, 10 Jan 2022 16:27:39 -0700 Subject: [PATCH 03/10] Try again to fix formatting --- doc/design/command_line_arguments.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/design/command_line_arguments.rst b/doc/design/command_line_arguments.rst index f9bcb91180..363b933577 100644 --- a/doc/design/command_line_arguments.rst +++ b/doc/design/command_line_arguments.rst @@ -25,6 +25,5 @@ This documents our conventions for command-line arguments, primarily as they per 4. For logical flags, use a flag without an argument -- ``--feature`` for the case where something is off by default and you want to turn it on, or ``--no-feature`` for the case where something is on by default and you want to turn it off -- instead of something like ``--feature true`` / ``--feature false``. Prefer phrasing these as positives whenever possible (e.g., ``--allow-multiple-pfts`` instead of ``--no-single-pft``). To the extent possible, try to clearly document what the behavior is if you add the flag, and what the behavior is if you don't add the flag. * Rationale: - * This use of ``--feature`` / ``--no-feature`` is more common behavior for Unix tools. * For something that is either on or off by default, you can see what the default operation is and what you can change just by looking at the available flag names, without needing to read through all of the documentation of default values. From a4cba427881e0a982f6e0fb72aa33d24939b0dbd Mon Sep 17 00:00:00 2001 From: Erik Kluzek Date: Tue, 11 Jan 2022 12:03:51 -0700 Subject: [PATCH 04/10] Update command_line_arguments.rst --- doc/design/command_line_arguments.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/design/command_line_arguments.rst b/doc/design/command_line_arguments.rst index 363b933577..f6e4709bfd 100644 --- a/doc/design/command_line_arguments.rst +++ b/doc/design/command_line_arguments.rst @@ -27,3 +27,4 @@ This documents our conventions for command-line arguments, primarily as they per * Rationale: * This use of ``--feature`` / ``--no-feature`` is more common behavior for Unix tools. * For something that is either on or off by default, you can see what the default operation is and what you can change just by looking at the available flag names, without needing to read through all of the documentation of default values. + * Whenever possible avoid the use of --no-feature arguments as they often are confusing. Try to express the same thing using positive wording. From a89bce2e46b09272b467b5b61fd46dae7e5bfd4e Mon Sep 17 00:00:00 2001 From: Erik Kluzek Date: Tue, 11 Jan 2022 12:16:16 -0700 Subject: [PATCH 05/10] Some changes based on suggestions by @negin513 and @wweider --- doc/design/command_line_arguments.rst | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/doc/design/command_line_arguments.rst b/doc/design/command_line_arguments.rst index f6e4709bfd..1197cae658 100644 --- a/doc/design/command_line_arguments.rst +++ b/doc/design/command_line_arguments.rst @@ -12,19 +12,33 @@ This documents our conventions for command-line arguments, primarily as they per Conventions for command-line arguments ======================================== -1. Options that are more than a single character should be formatted as ``--some-variable`` -- i.e., with a double leading hyphen and a hyphen as word separator. +1. Longer options: + +Options that are more than a single character should be formatted as ``--some-variable`` -- i.e., with a double leading hyphen and a hyphen as word separator. * Rationale: This follows the `POSIX conventions `_. -2. Scripts should support ``--verbose`` / ``-v`` and ``--debug`` options. See comments at the top of ``ctsm_logging.py`` for details. +2. Standard options: verbose, silent, help and debug: + +Scripts should support ``--verbose`` / ``-v`` and ``--debug`` options. See comments at the top of ``ctsm_logging.py`` for details. +Also the options silent and help are recommended as well. -3. For arguments that need a value: If there is no obvious default for the value, then the argument should be required rather than being given a default that the user will often need to override. A general rule of thumb is: if, say, 75% -- 90% of users will want a particular value, then it can be okay to have this value as the default; otherwise, it should be a required argument. This holds even for arguments that are only needed in certain circumstances: For example, if argument ``--fooval FOOVAL`` needs to be given whenever ``--bar`` is given, but there is no obvious default for ``FOOVAL``, then the argument parsing should be coded so that ``--fooval`` does not have a default, and there is error checking code that ensures that ``fooval`` has been specified (not ``None``) if ``--bar`` was given. +3. Value flags: + +For arguments that need a value: If there is no obvious default for the value, then the argument should be required rather than being given a default that the user will often need to override. A general rule of thumb is: if, say, 75% -- 90% of users will want a particular value, then it can be okay to have this value as the default; otherwise, it should be a required argument. This holds even for arguments that are only needed in certain circumstances: For example, if argument ``--fooval FOOVAL`` needs to be given whenever ``--bar`` is given, but there is no obvious default for ``FOOVAL``, then the argument parsing should be coded so that ``--fooval`` does not have a default, and there is error checking code that ensures that ``fooval`` has been specified (not ``None``) if ``--bar`` was given. * Rationale: Giving an argument a default value means that we can't catch and report when a user forgets to set it but should have set it. Leaving off the default allows for this error checking. + * Example: To set the start year for DATM forcing... + --datm-syr 1850 + +4. Switch flags: -4. For logical flags, use a flag without an argument -- ``--feature`` for the case where something is off by default and you want to turn it on, or ``--no-feature`` for the case where something is on by default and you want to turn it off -- instead of something like ``--feature true`` / ``--feature false``. Prefer phrasing these as positives whenever possible (e.g., ``--allow-multiple-pfts`` instead of ``--no-single-pft``). To the extent possible, try to clearly document what the behavior is if you add the flag, and what the behavior is if you don't add the flag. +For logical flags, use a flag without an argument -- ``--feature`` for the case where something is off by default and you want to turn it on, or ``--no-feature`` for the case where something is on by default and you want to turn it off -- instead of something like ``--feature true`` / ``--feature false``. Prefer phrasing these as positives whenever possible (e.g., ``--allow-multiple-pfts`` instead of ``--no-single-pft``). To the extent possible, try to clearly document what the behavior is if you add the flag, and what the behavior is if you don't add the flag. * Rationale: * This use of ``--feature`` / ``--no-feature`` is more common behavior for Unix tools. * For something that is either on or off by default, you can see what the default operation is and what you can change just by looking at the available flag names, without needing to read through all of the documentation of default values. * Whenever possible avoid the use of --no-feature arguments as they often are confusing. Try to express the same thing using positive wording. + * Examples: To turn prognostic crop on (or off)... + --crop + --no-crop From 23be0fde202ab6e6ae4b3219ddcba06d647d1305 Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Wed, 12 Jan 2022 18:02:50 -0700 Subject: [PATCH 06/10] Rename file to be more general --- ...ommand_line_arguments.rst => python_script_user_interface.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename doc/design/{command_line_arguments.rst => python_script_user_interface.rst} (100%) diff --git a/doc/design/command_line_arguments.rst b/doc/design/python_script_user_interface.rst similarity index 100% rename from doc/design/command_line_arguments.rst rename to doc/design/python_script_user_interface.rst From a3b0a379b825c0a5f91b308eefc66bb3760c71a8 Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Thu, 13 Jan 2022 07:28:59 -0700 Subject: [PATCH 07/10] Add some additional sections to python script design doc Thank you to @negin513 for the good suggestions of adding these sections. --- doc/design/python_script_user_interface.rst | 35 +++++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/doc/design/python_script_user_interface.rst b/doc/design/python_script_user_interface.rst index 1197cae658..e3ca474f58 100644 --- a/doc/design/python_script_user_interface.rst +++ b/doc/design/python_script_user_interface.rst @@ -6,7 +6,25 @@ Overview of this design document ================================== -This documents our conventions for command-line arguments, primarily as they pertain to python scripts. These primarily focus on user-visible aspects, as opposed to implementation details. +This documents various conventions for the user interface of python scripts. The focus here is on user-visible aspects, as opposed to implementation details. + +==================================================== + Separation of front-end script from implementation +==================================================== + +Most python code resides in the ``python/ctsm`` directory, and modules there have a ``.py`` extension. However, scripts that are run from the command-line have small wrapper files that reside in appropriate places throughout the repository (e.g., in the ``tools`` directory). These wrapper files should *not* have a ``.py`` extension, and should contain as little python code as possible (since these files aren't checked by pylint and cannot easily be unit tested): typically they contain just enough code to setup the python path, load a python module and then call the main function from that module. See examples throughout CTSM for details. + +Rationale: Modules meant to be imported (i.e., everything under ``python/ctsm``) should have a ``.py`` extension. However, it is valuable to keep the extension off of the scripts that users run for a few reasons: +1. A user shouldn't need to know what language a script is in when all they want to do is to run the script +2. We want to avoid the need for retraining users, updating documentation, etc. if we change the implementation language +3. Standard Unix utilities rarely if ever require specifying the language extension when running them; the same is true for the core scripts in CIME (``create_newcase``, ``xmlchange``, etc.); we would like to stay consistent with these other scripts and utilities + +Counter-arguments: Arguments for keeping a ``.py`` extension even on the files meant to be run by users are the following: +1. It's obvious from looking at a directory listing what language a file is in +2. This extension may be needed on Windows systems +3. The file extension is needed to support linting, unit testing, etc. + +Although there are good arguments on both sides, the stability of the user experience is supported by excluding the language extension. We choose to prioritize the user experience over developer convenience. (The possible inability of running on Windows systems does not currently feel like an issue, because users don't typically try to run these scripts on Windows systems as far as we know.) ======================================== Conventions for command-line arguments @@ -28,8 +46,7 @@ Also the options silent and help are recommended as well. For arguments that need a value: If there is no obvious default for the value, then the argument should be required rather than being given a default that the user will often need to override. A general rule of thumb is: if, say, 75% -- 90% of users will want a particular value, then it can be okay to have this value as the default; otherwise, it should be a required argument. This holds even for arguments that are only needed in certain circumstances: For example, if argument ``--fooval FOOVAL`` needs to be given whenever ``--bar`` is given, but there is no obvious default for ``FOOVAL``, then the argument parsing should be coded so that ``--fooval`` does not have a default, and there is error checking code that ensures that ``fooval`` has been specified (not ``None``) if ``--bar`` was given. * Rationale: Giving an argument a default value means that we can't catch and report when a user forgets to set it but should have set it. Leaving off the default allows for this error checking. - * Example: To set the start year for DATM forcing... - --datm-syr 1850 + * Example: To set the start year for DATM forcing: ``--datm-syr 1850`` 4. Switch flags: @@ -39,6 +56,12 @@ For logical flags, use a flag without an argument -- ``--feature`` for the case * This use of ``--feature`` / ``--no-feature`` is more common behavior for Unix tools. * For something that is either on or off by default, you can see what the default operation is and what you can change just by looking at the available flag names, without needing to read through all of the documentation of default values. * Whenever possible avoid the use of --no-feature arguments as they often are confusing. Try to express the same thing using positive wording. - * Examples: To turn prognostic crop on (or off)... - --crop - --no-crop + * Examples: To turn prognostic crop on (or off): ``--crop`` or ``--no-crop`` + +========= + Logging +========= + +We try to follow the guide at the top of `Python's logging howto `_. In particular, print statements should be used for "console output for ordinary usage of a command line script or program"; ``logger.info`` or ``logger.debug`` should be used to "report events that occur during normal operation of a program (e.g. for status monitoring or fault investigation)", etc. The distinction between when to use print and when to use logging can admittedly be a bit subjective, as it comes down to the question of whether the given output is part of the fundamental operation of the script – i.e., part of what the script is designed to do is to give this output. For example, ``run_sys_tests`` prints a variety of information when it starts, particularly concerning the git and manage_externals status of the current repository. The rationale for using ``print`` statements for this is that Bill designed ``run_sys_tests`` to replace some of the repetitive items that he did whenever running the system tests. One of these items was running ``git status`` and ``./manage_externals/checkout_externals -S -v`` to check that the repository is in a clean state. Thus, in this case, Bill's view is that the output from these commands is part of the fundamental purpose of ``run_sys_tests``: it is something he always wants to see, and he feels that it is important for anyone running the system tests to review, and thus ``print`` statements are appropriate here. + +Near the top of each python module where logging is used, there should be a line, ``logger = logging.getLogger(__name__)``. Then logging statements should be done using statements like ``logger.info(...)``, *not* ``logging.info(...)``: this allows more contextual information in logging output. From 7fd747a7be29e3b8d1ba57811d7e651b32070d09 Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Thu, 13 Jan 2022 07:32:07 -0700 Subject: [PATCH 08/10] Fix(?) formatting --- doc/design/python_script_user_interface.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/design/python_script_user_interface.rst b/doc/design/python_script_user_interface.rst index e3ca474f58..915ebee2a1 100644 --- a/doc/design/python_script_user_interface.rst +++ b/doc/design/python_script_user_interface.rst @@ -9,17 +9,19 @@ This documents various conventions for the user interface of python scripts. The focus here is on user-visible aspects, as opposed to implementation details. ==================================================== - Separation of front-end script from implementation + Separation of front-end scripts from implementation ==================================================== Most python code resides in the ``python/ctsm`` directory, and modules there have a ``.py`` extension. However, scripts that are run from the command-line have small wrapper files that reside in appropriate places throughout the repository (e.g., in the ``tools`` directory). These wrapper files should *not* have a ``.py`` extension, and should contain as little python code as possible (since these files aren't checked by pylint and cannot easily be unit tested): typically they contain just enough code to setup the python path, load a python module and then call the main function from that module. See examples throughout CTSM for details. Rationale: Modules meant to be imported (i.e., everything under ``python/ctsm``) should have a ``.py`` extension. However, it is valuable to keep the extension off of the scripts that users run for a few reasons: + 1. A user shouldn't need to know what language a script is in when all they want to do is to run the script 2. We want to avoid the need for retraining users, updating documentation, etc. if we change the implementation language 3. Standard Unix utilities rarely if ever require specifying the language extension when running them; the same is true for the core scripts in CIME (``create_newcase``, ``xmlchange``, etc.); we would like to stay consistent with these other scripts and utilities Counter-arguments: Arguments for keeping a ``.py`` extension even on the files meant to be run by users are the following: + 1. It's obvious from looking at a directory listing what language a file is in 2. This extension may be needed on Windows systems 3. The file extension is needed to support linting, unit testing, etc. From 1091be6965ff928b3205e6a2c95f82b7ebb66cc5 Mon Sep 17 00:00:00 2001 From: Bill Sacks Date: Wed, 26 Jan 2022 11:42:39 -0700 Subject: [PATCH 09/10] Add some clarifications regarding use of prints vs logging --- doc/design/python_script_user_interface.rst | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/doc/design/python_script_user_interface.rst b/doc/design/python_script_user_interface.rst index 915ebee2a1..535c2470e9 100644 --- a/doc/design/python_script_user_interface.rst +++ b/doc/design/python_script_user_interface.rst @@ -64,6 +64,15 @@ For logical flags, use a flag without an argument -- ``--feature`` for the case Logging ========= -We try to follow the guide at the top of `Python's logging howto `_. In particular, print statements should be used for "console output for ordinary usage of a command line script or program"; ``logger.info`` or ``logger.debug`` should be used to "report events that occur during normal operation of a program (e.g. for status monitoring or fault investigation)", etc. The distinction between when to use print and when to use logging can admittedly be a bit subjective, as it comes down to the question of whether the given output is part of the fundamental operation of the script – i.e., part of what the script is designed to do is to give this output. For example, ``run_sys_tests`` prints a variety of information when it starts, particularly concerning the git and manage_externals status of the current repository. The rationale for using ``print`` statements for this is that Bill designed ``run_sys_tests`` to replace some of the repetitive items that he did whenever running the system tests. One of these items was running ``git status`` and ``./manage_externals/checkout_externals -S -v`` to check that the repository is in a clean state. Thus, in this case, Bill's view is that the output from these commands is part of the fundamental purpose of ``run_sys_tests``: it is something he always wants to see, and he feels that it is important for anyone running the system tests to review, and thus ``print`` statements are appropriate here. +We try to follow the guide at the top of `Python's logging howto `_. In particular, print statements should be used for "console output for ordinary usage of a command line script or program"; ``logger.info`` or ``logger.debug`` should be used to "report events that occur during normal operation of a program (e.g. for status monitoring or fault investigation)", etc. + +The distinction between when to use print and when to use logging can admittedly be a bit subjective, as it comes down to the question of whether the given output is part of the fundamental operation of the script – i.e., part of what the script is designed to do is to give this output. For example, ``run_sys_tests`` prints a variety of information when it starts, particularly concerning the git and manage_externals status of the current repository. The rationale for using ``print`` statements for this is that we designed ``run_sys_tests`` to replace some of the repetitive items that we did whenever running the system tests. One of these items was running ``git status`` and ``./manage_externals/checkout_externals -S -v`` to check that the repository is in a clean state. Thus, in this case, our view is that the output from these commands is part of the fundamental purpose of ``run_sys_tests``: it is something we always want to see, and we feel that it is important for anyone running the system tests to review, and thus ``print`` statements are appropriate here. + +In general, ``print`` statements should be used sparingly, just for output that is important for the user to see. That said, users of CTSM scripts often expect more output than you would see from a typical Unix tool (where the philosophy is that there should be no output if everything worked correctly). Some examples of things that users of CTSM scripts typically want to see are: + +* A final "success" message at the end of the script +* Paths to directories or files created by the script + +More verbose output should go in ``logger.info`` statements (or ``logger.debug`` statements for output that a normal user would rarely want to see). Near the top of each python module where logging is used, there should be a line, ``logger = logging.getLogger(__name__)``. Then logging statements should be done using statements like ``logger.info(...)``, *not* ``logging.info(...)``: this allows more contextual information in logging output. From 038df8d15a3cb0f2c9403b16f21c19d293b2c1dc Mon Sep 17 00:00:00 2001 From: Adrianna Foster Date: Thu, 27 Jan 2022 16:04:29 -0700 Subject: [PATCH 10/10] update for info vs. debug --- doc/design/python_script_user_interface.rst | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/design/python_script_user_interface.rst b/doc/design/python_script_user_interface.rst index 535c2470e9..4dd0c9c546 100644 --- a/doc/design/python_script_user_interface.rst +++ b/doc/design/python_script_user_interface.rst @@ -12,7 +12,7 @@ This documents various conventions for the user interface of python scripts. The Separation of front-end scripts from implementation ==================================================== -Most python code resides in the ``python/ctsm`` directory, and modules there have a ``.py`` extension. However, scripts that are run from the command-line have small wrapper files that reside in appropriate places throughout the repository (e.g., in the ``tools`` directory). These wrapper files should *not* have a ``.py`` extension, and should contain as little python code as possible (since these files aren't checked by pylint and cannot easily be unit tested): typically they contain just enough code to setup the python path, load a python module and then call the main function from that module. See examples throughout CTSM for details. +Most python code resides in the ``python/ctsm`` directory, and modules there have a ``.py`` extension. However, scripts that are run from the command-line have small wrapper files that reside in appropriate places throughout the repository (e.g., in the ``tools`` directory). These wrapper files should *not* have a ``.py`` extension, and should contain as little python code as possible (since these files aren't checked by pylint and cannot easily be unit tested): typically they contain just enough code to setup the python path, load a python module, and then call the main function from that module. See examples throughout CTSM for details. Rationale: Modules meant to be imported (i.e., everything under ``python/ctsm``) should have a ``.py`` extension. However, it is valuable to keep the extension off of the scripts that users run for a few reasons: @@ -73,6 +73,11 @@ In general, ``print`` statements should be used sparingly, just for output that * A final "success" message at the end of the script * Paths to directories or files created by the script -More verbose output should go in ``logger.info`` statements (or ``logger.debug`` statements for output that a normal user would rarely want to see). +More verbose output should go in ``logger.info`` or ``logger.debug`` statements for output that a normal user would rarely want to see. The difference between output that should go in ``logger.info`` vs. ``logger.debug`` statements is somewhat subjective, but some general rules are: + +* INFO statements include high-level, informational statements about the program state, the user, which files are being used and where, etc. + * e.g. A ``logging.info`` statement might be ``logger.info("Reading in file %s from %s", file_name, dir_name)``. +* DEBUG statements include fine-grained statements about program state, that really only developers digging into the code would want to see. + * e.g. You might want to output a ``logging.debug`` statement for every variable in a file you are editing. Near the top of each python module where logging is used, there should be a line, ``logger = logging.getLogger(__name__)``. Then logging statements should be done using statements like ``logger.info(...)``, *not* ``logging.info(...)``: this allows more contextual information in logging output.