help info for parameters appears in cli for upload and download by Bento007 · Pull Request #146 · HumanCellAtlas/dcp-cli

Bento007 · 2018-07-31T02:37:27Z

Release Notes

Using sphinx-argparser to generate documentation for the CLI into cli.rst
refactored docstrings to restructed text format
TOC now populates in sphinx documentation
added common links to epilog in sphinx documentation
using sphinx RSTParser to parse docstrings into argparser
spread out documentation across api.rst, readme.rst, cli.rst, and index.rst

Known Issue

the sphinx documentation incorrectly marks all commands generated from a swagger doc as a classmethod.

codecov-io · 2018-07-31T16:13:07Z

Codecov Report

Merging #146 into master will increase coverage by 0.61%.
The diff coverage is 97.22%.

@@            Coverage Diff             @@
##           master     #146      +/-   ##
==========================================
+ Coverage   86.75%   87.37%   +0.61%     
==========================================
  Files          33       33              
  Lines        1208     1291      +83     
==========================================
+ Hits         1048     1128      +80     
- Misses        160      163       +3

Impacted Files	Coverage Δ
hca/upload/__init__.py	`93.75% <ø> (ø)`	⬆️
hca/cli.py	`66.21% <100%> (+1.42%)`	⬆️
hca/dss/__init__.py	`83.64% <100%> (ø)`	⬆️
hca/util/__init__.py	`93.11% <100%> (+0.02%)`	⬆️
hca/util/_docs.py	`96.96% <96%> (-3.04%)`	⬇️
hca/upload/upload_config.py	`90.9% <0%> (-0.9%)`	⬇️
hca/upload/upload_area.py	`98.13% <0%> (+1.02%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 694c5b9...722968a. Read the comment docs.

hannes-ucsc

@Bento007 and I talked in person and agreed on some changes. Rejecting this purely for procedural reasons so I get notified for the next round of reviews.

* added cli.rst to generate cli documentation. * added epilog to all sphinx generated pages.

hannes-ucsc

Neat.

hannes-ucsc · 2018-08-03T17:54:44Z

+
+    @staticmethod
+    def parse_docstring(docstring):
+        """


If you use triple single quotes here (despite that being a PEP8 violation), …

hannes-ucsc · 2018-08-03T17:55:15Z

+        arguments. Any other text is combined and added to the argparse description.
+
+        example:
+        \"\"\"


maybe you can use triple double quotes here. Would look less odd.

kislyuk · 2018-08-03T20:09:06Z

-* :ref:`search`
+   readme
+   cli
+   api


This makes the index page only contain a table of contents. Please change this so that the introduction still appears on the index page.

added readme.rst to index.rst

kislyuk · 2018-08-03T20:11:53Z

-        manifest is not set. If and only if a data file matches any of the patterns in `data_files` will it be
-        downloaded.
+        :param str bundle_uuid: The uuid of the bundle to download
+        :param str replica: the replica to download from. [aws,gcp,azure]


Please remove "azure" and spell out the possible values.

removed azure and specified what the replicas mean.

kislyuk · 2018-08-03T20:12:23Z

-        downloaded.
+        :param str bundle_uuid: The uuid of the bundle to download
+        :param str replica: the replica to download from. [aws,gcp,azure]
+        :param str version: Timestamp of bundle creation in RFC3339. The version to download, else download the latest.


s/else/If not specified/

kislyuk · 2018-08-03T20:12:59Z

-        Each row in the manifest represents one file in DSS. The manifest must have a header row. The header row must
-        declare the following columns:
+        :param str manifest: path to a TSV (tab-separated values) file listing files to download
+        :param str replica: the replica to download from. [aws,gcp,azure]


Same as above

kislyuk · 2018-08-03T20:13:12Z

-        declare the following columns:
+        :param str manifest: path to a TSV (tab-separated values) file listing files to download
+        :param str replica: the replica to download from. [aws,gcp,azure]
+        :param int initial_retries_left: The initial number of times to retries a failed download.


s/retries/retry

Also, what does "initial" mean here? It should really be "num_retries" or something else more intuitive.

added additional details to initial_retries_left

kislyuk · 2018-08-03T20:14:02Z

        Upload a directory of files from the local filesystem and create a bundle containing the uploaded files.

+        :param str src_dir: file path to a directory of file to upload
+        :param str replica: the replica to upload to. [aws,gcp,azure]


Same as above.

kislyuk · 2018-08-03T20:14:14Z

+        :param str src_dir: file path to a directory of file to upload
+        :param str replica: the replica to upload to. [aws,gcp,azure]
+        :param str staging_bucket: the bucket to upload from.
+        :param int timeout_seconds: timeout.


Timeout for what?

added more details.

kislyuk · 2018-08-03T20:14:23Z


+        :param str src_dir: file path to a directory of file to upload
+        :param str replica: the replica to upload to. [aws,gcp,azure]
+        :param str staging_bucket: the bucket to upload from.


Bucket where?

added more details.

kislyuk · 2018-08-03T20:17:34Z

 backports.tempfile
 mock
 unittest2
+sphinx-argparse


requirements-dev is for test/development-only requirements, but you are importing these packages in the main codebase. So you have to put this in requirements.txt, not requirements-dev.

Also, when I installed this, it didn't seem to automatically pull in sphinx, and I got an error:

>make docs /Library/Developer/CommandLineTools/usr/bin/make -C docs html Running Sphinx v1.5.5 Exception occurred: File "/Users/akislyuk/projects/dcp-cli/hca/util/__init__.py", line 100, in <module> from sphinx.parsers import RSTParser ImportError: cannot import name RSTParser

I ran pip install sphinx and the error went away. Can you check if sphinx-argparse pulls in sphinx? If not, you have to list it here.

added docutils to requirements.txt, added sphinx and sphinx-argparser to requirement-dev.txt

kislyuk · 2018-08-03T20:18:43Z

                                               **self._get_command_arg_settings(param_data))
+
+    @staticmethod
+    def parse_docstring(docstring):


To make package organization more consistent and uncrowd this file, please detach this method and move it to hca/util/_docs.py.

moved to hca/util/_docs.py

* moved docstrings for API to __init__ files. * deleted readme.rst * README.rst appears above table of contents in index.rst page. * removed unused imports from docs/conf.py * changed sphinx project name from 'hca-cli' to 'HCA CLI' * change epilog format to look less confusing.

… and staging_bucket.

…cific parsing.

kislyuk · 2018-08-06T22:56:52Z

+            set. If and only if a metadata file matches any of the patterns in `metadata_files` will it be downloaded.
+        :param list data_files: one or more shell patterns against which all data files in the bundle will be matched
+            case-sensitively. A file is considered a data file if the `indexed` property in the manifest is not set. If
+            and only if a data file matches any of the patterns in `data_files` will it be downloaded.


Style: "The file will be downloaded only if ..."

kislyuk · 2018-08-06T22:58:23Z

+        :param list data_files: one or more shell patterns against which all data files in the bundle will be matched
+            case-sensitively. A file is considered a data file if the `indexed` property in the manifest is not set. If
+            and only if a data file matches any of the patterns in `data_files` will it be downloaded.
+        :param int initial_retries_left: The initial quota of download failures to accept before exiting due to


This parameter should be renamed to something like num_retries. "Initial" and "left" are implementation details.

"The number of times to attempt the download before exiting with an error"

Bento007 self-assigned this Jul 31, 2018

Bento007 requested a review from kislyuk July 31, 2018 02:37

Bento007 force-pushed the tsmith-cli-doc branch from bf5b4aa to 8c593b6 Compare July 31, 2018 02:38

Bento007 requested a review from hannes-ucsc July 31, 2018 02:42

Bento007 force-pushed the tsmith-cli-doc branch 3 times, most recently from 6a04d6f to 9a1eb4c Compare August 1, 2018 22:56

hannes-ucsc requested changes Aug 2, 2018

View reviewed changes

Trent Smith added 3 commits August 2, 2018 19:54

auto generating documentation for the CLI in using sphinx-argparser

590aa3c

* added cli.rst to generate cli documentation. * added epilog to all sphinx generated pages.

refactored docstring to rst format in hca.dss commands

d484a06

adding api and readme as separate rst file to the sphinx documentation

f66b0df

Bento007 force-pushed the tsmith-cli-doc branch from 6b69d6b to 8bb7827 Compare August 3, 2018 02:54

Using RSTParser to parse docstrings into argparser

2b33caa

Bento007 force-pushed the tsmith-cli-doc branch from 8bb7827 to 2b33caa Compare August 3, 2018 03:24

Bento007 requested a review from hannes-ucsc August 3, 2018 03:28

hannes-ucsc approved these changes Aug 3, 2018

View reviewed changes

chaning quotes on parse_docstring docstring

aaff6cd

kislyuk requested changes Aug 3, 2018

View reviewed changes

Trent Smith added 2 commits August 3, 2018 16:56

moving _parse_docstring to hca.utils._doc

6fb77e2

Bento007 force-pushed the tsmith-cli-doc branch from 7b28fe7 to 6fb77e2 Compare August 4, 2018 05:51

Trent Smith added 3 commits August 6, 2018 08:43

adding more detail to initial_retries_left, replica, timeout_seconds,…

c713aec

… and staging_bucket.

using docutil over sphinx since we aren't using any of the sphinx spe…

5de4546

…cific parsing.

adding sphinx and docutils to requirements

4e54296

Bento007 requested a review from kislyuk August 6, 2018 16:14

kislyuk reviewed Aug 6, 2018

View reviewed changes

kislyuk approved these changes Aug 6, 2018

View reviewed changes

renamed initial_retries_left to num_retries

722968a

Bento007 merged commit aa81149 into master Aug 8, 2018

Bento007 deleted the tsmith-cli-doc branch August 8, 2018 20:28

kozbo mentioned this pull request Aug 9, 2018

Add command line positional args for both upload and dss to the HCA documentation page. #129

Closed

kislyuk mentioned this pull request Feb 5, 2019

Fix DSS API documentation #227

Merged

DailyDreaming mentioned this pull request Feb 27, 2019

CLI Help Menu Formatting #219

Closed

Conversation

Bento007 commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Notes

Known Issue

Uh oh!

codecov-io commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hannes-ucsc left a comment

Choose a reason for hiding this comment

Uh oh!

hannes-ucsc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Bento007 commented Jul 31, 2018 •

edited

Loading

codecov-io commented Jul 31, 2018 •

edited

Loading