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

DOC: Extend description for delete to point that it could be URL etc #609

Merged
merged 1 commit into from
May 3, 2021

Conversation

yarikoptic
Copy link
Member

isn't that right @jwodder ?

@yarikoptic yarikoptic added UX patch Increment the patch version when merged labels Apr 30, 2021
@yarikoptic yarikoptic requested a review from jwodder April 30, 2021 22:21
@codecov
Copy link

codecov bot commented Apr 30, 2021

Codecov Report

Merging #609 (2c498c4) into master (914a9cf) will increase coverage by 1.57%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #609      +/-   ##
==========================================
+ Coverage   83.08%   84.66%   +1.57%     
==========================================
  Files          59       59              
  Lines        5681     6109     +428     
==========================================
+ Hits         4720     5172     +452     
+ Misses        961      937      -24     
Flag Coverage Δ
unittests 84.66% <ø> (+1.57%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dandi/cli/cmd_delete.py 81.81% <ø> (ø)
dandi/consts.py 100.00% <0.00%> (ø)
dandi/tests/test_utils.py 100.00% <0.00%> (ø)
dandi/cli/tests/test_ls.py 100.00% <0.00%> (ø)
dandi/tests/test_dandiarchive.py 100.00% <0.00%> (ø)
dandi/dandiset.py 85.71% <0.00%> (+1.19%) ⬆️
dandi/download.py 87.73% <0.00%> (+1.92%) ⬆️
dandi/delete.py 93.78% <0.00%> (+4.17%) ⬆️
dandi/dandiarchive.py 85.41% <0.00%> (+5.14%) ⬆️
dandi/cli/base.py 97.36% <0.00%> (+5.26%) ⬆️
... and 3 more

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 914a9cf...2c498c4. Read the comment docs.

@@ -10,7 +10,11 @@
@devel_debug_option()
@map_to_click_exceptions
def delete(paths, skip_missing, dandi_instance="dandi", devel_debug=False):
""" Delete Dandisets and assets from the server """
"""Delete Dandisets and assets from the server
Copy link
Member

Choose a reason for hiding this comment

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

There needs to be a period at the end of this line to terminate the short description shown in dandi --help.

Copy link
Member Author

Choose a reason for hiding this comment

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

THANKS!

"""Delete Dandisets and assets from the server

PATH could be a local path or a URL to an asset, directory, or an entire
dandiset.
Copy link
Member

Choose a reason for hiding this comment

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

"Dandiset" should be capitalized, shouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

For consistency with prior summary one -- sure... but it seems that we do not have consistency in general, but other commands use it lowercased. So I will make it lower cased here as well and it would look like

Usage: dandi [OPTIONS] COMMAND [ARGS]...

  A client to support interactions with DANDI archive
  (http://dandiarchive.org).

  To see help for a specific command, run

      dandi COMMAND --help

  e.g. dandi upload --help

Options:
  --version
  -l, --log-level [DEBUG|INFO|WARNING|ERROR|CRITICAL]
                                  Log level (case insensitive).  May be
                                  specified as an integer.  [default: INFO]

  --pdb                           Fall into pdb if errors out
  --help                          Show this message and exit.

Commands:
  delete    Delete dandisets and assets from the server.
  digest    Calculate file digests
  download  Download a file or entire folder from DANDI
  ls        List .nwb files and dandisets metadata.
  organize  (Re)organize files according to the metadata.
  upload    Upload dandiset (files) to DANDI archive.
  validate  Validate files for NWB (and DANDI) compliance.

@yarikoptic
Copy link
Member Author

I have addressed the comments, let's proceed. Thanks @jwodder for the review!

@yarikoptic yarikoptic merged commit 58f30a6 into master May 3, 2021
@yarikoptic yarikoptic deleted the enh-doc-delete branch May 3, 2021 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants