Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding descriptions to two settings #1733

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

john-science
Copy link
Member

@john-science john-science commented Jun 16, 2024

What is the change?

Adding descriptions to two settings: CONF_COPY_FILES_FROM and CONF_COPY_FILES_TO.

Also, while I was at it, this work let me to:

  • Improve the docstring on the method MainInterface._moveFiles()
  • Add a DeprecationWarning that soon settings will require descriptions (same as with parameters).
  • I fixed some formatting and docstrings as I found them (sorry!)

Why is the change being made?

To close #1727

These two settings are slightly complicated, and interact heavily, so they should have descriptions that explain their use.


Checklist

  • This PR has only one purpose or idea. (Except the random cleanup I did. Sorry!)
  • Tests have been added/updated to verify any new/changed code.
  • The release notes have been updated if necessary.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in pyproject.toml.

@john-science john-science added the documentation Improvements or additions to documentation label Jun 16, 2024
armi/settings/setting.py Outdated Show resolved Hide resolved
Comment on lines 102 to 112
- List of files to copy (cannot be directories).
- Can be of length zero (that just means no files will be copied).
- The file names listed can use the ``*`` glob syntax, to reference multiple files.


``CONF_COPY_FILES_TO`` :

- List of directories to copy the files into.
- Can be of length zero; all files will be copied to the local dir.
- Can be of length one; all files will be copied to that dir.
- The only other valid length for this list _must_ be the same length as the "from" list.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all really useful context for a user. Unfortunately users won't typically see this info because it is in the docstring of a private method. Is there a better place to put this that will make it user-facing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I can' think of where else it would belong.

armi/bookkeeping/mainInterface.py Outdated Show resolved Hide resolved
@john-science john-science merged commit de4eeee into terrapower:main Jun 18, 2024
12 checks passed
@john-science john-science deleted the copy_files_descs branch June 18, 2024 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add descriptions for copyFilesFrom and copyFilesTo case settings
2 participants