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

Add docstrings for cubids/cli.py #391

Merged
merged 19 commits into from
Jan 16, 2025
Merged

Add docstrings for cubids/cli.py #391

merged 19 commits into from
Jan 16, 2025

Conversation

singlesp
Copy link
Contributor

@singlesp singlesp commented Jan 16, 2025

relates to #384 and #358.

Changes proposed in this pull request

  • add docstrings to cli.py
  • consistent single back-ticks throughout when referring to commands
  • docstrings and descriptions for parse functions now refer exclusively to the new command format
  • all entry functions now trigger a deprecation warning
  • module docstring added

@singlesp
Copy link
Contributor Author

entry point cli arguments raise a deprecation error, but the corresponding arg parsers do not. Should we add mention of deprecation in the corresponding arg parsers?

cubids/cli.py Outdated
_main(argv=None)
Set entrypoint for "cubids" CLI.

Each function that serves as an entry point for a CLI command in the formart of `cubids-function`
Copy link
Contributor Author

@singlesp singlesp Jan 16, 2025

Choose a reason for hiding this comment

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

Check this wording + see related comment on the conversation page.

@singlesp
Copy link
Contributor Author

singlesp commented Jan 16, 2025

  • add the command that is called to the summary statement for entry and parse functions.
  • all entry functions should have a deprecation warning.
  • use correct tense (parse instead of parses).
  • use single-back ticks to emphasize a code line (cli.py is not being rendered on the docs so double ins't needed)
  • fix lint formatting

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

Just a few small things

cubids/cli.py Outdated Show resolved Hide resolved
cubids/cli.py Outdated Show resolved Hide resolved
cubids/cli.py Outdated Show resolved Hide resolved
cubids/cli.py Outdated
Comment on lines 310 to 320
Returns
-------
argparse.ArgumentParser
The argument parser with the necessary arguments configured.

Parameters
----------
from_json : str
Source JSON file path. This file contains the data to be copied.
to_json : str
Destination JSON file path. This file will have data from `from_json` copied into it.
Copy link
Member

Choose a reason for hiding this comment

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

Can you put Parameters before Returns in these?

Copy link
Member

Choose a reason for hiding this comment

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

It looks like it's still backwards in this one.

@singlesp singlesp requested a review from tsalo January 16, 2025 21:14
@tsalo tsalo changed the title docstrings for cubids/cli.py Add docstrings for cubids/cli.py Jan 16, 2025
@tsalo tsalo added the documentation Improvements or additions to documentation label Jan 16, 2025
@singlesp singlesp merged commit ef98d4f into main Jan 16, 2025
10 checks passed
@singlesp singlesp deleted the docstrings-parker branch January 16, 2025 21:24
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.

2 participants