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.

Sign up for GitHub

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

Jump to bottom

ENH+BF: make it possible to export_to_figshare without cd'ing #2723

Merged
merged 4 commits into from Aug 3, 2018
Merged

ENH+BF: make it possible to export_to_figshare without cd'ing #2723

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
337768b
ENH+BF: make it possible to export_to_figshare without cd'ing
yarikoptic Jul 31, 2018
0d54343
DOC: export_archive: Reword PATH's description
kyleam Aug 1, 2018
ab7fe17
RF(minor): renamed a variable and added information about destination…
yarikoptic Aug 1, 2018
231f6b2
BF(PY3): list the map!
yarikoptic Aug 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions datalad/plugin/export_archive.py
View file
Open in desktop
Expand Up @@ -12,7 +12,7 @@

from datalad.interface.base import Interface
from datalad.interface.base import build_doc

from datalad.support import path

@build_doc
class ExportArchive(Interface):
Expand All @@ -37,7 +37,9 @@ class ExportArchive(Interface):
nargs='?',
doc="""File name of the generated TAR archive. If no file name is
given the archive will be generated in the current directory and
will be named: datalad_<dataset_uuid>.(tar.*|zip).""",
will be named: datalad_<dataset_uuid>.(tar.*|zip). To generate that
file in a different directory, provide an existing directory as the
file name.""",
constraints=EnsureStr() | EnsureNone()),
archivetype=Parameter(
args=("-t", "--archivetype"),
Expand Down Expand Up @@ -106,8 +108,11 @@ def _filter_tarinfo(ti):
'.' if compression else '',
compression) if archivetype == 'tar' else '')

filename_tmpl = "datalad_{.id}".format(dataset)
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this name confusing because I assume "tmpl" is for "template", but it isn't used as a template for the file name. It's just the base name.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, indeed... I guess I had some visions of making it into cmdline option template so we could provide dataset etc objects for it to get filled in. renaming to default_filename

if filename is None:
filename = "datalad_{}".format(dataset.id)
filename = filename_tmpl # in current directory
elif path.exists(filename) and path.isdir(filename):
filename = path.join(filename, filename_tmpl) # under given directory
if not filename.endswith(file_extension):
filename += file_extension

Expand Down
10 changes: 7 additions & 3 deletions datalad/plugin/export_to_figshare.py
View file
Open in desktop
Expand Up @@ -165,8 +165,8 @@ class ExportToFigshare(Interface):
metavar="PATH",
nargs='?',
doc="""File name of the generated ZIP archive. If no file name is
given the archive will be generated in the current directory and
will be named: datalad_<dataset_uuid>.zip.""",
given the archive will be generated in the top directory
of the dataset and will be named: datalad_<dataset_uuid>.zip.""",
Copy link
Contributor

Choose a reason for hiding this comment

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

To be consistent with other places in datalad, shouldn't this be the top-level of the dataset if dataset is explicitly specified (via datalad -d or ds.export_to_figshare) and the current directory otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

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

export_archive is typically used to "export" it to the drive outside of the dataset e.g. for archival or sharing with a buddy, and we would use it with -d or just as a method of a dataset if under python. We wouldn't want it typically to be generated under dataset directory IMHO.
figshare and alike, where we need to upload it and desire to addurl it back, so it should be exported under dataset is atypical imho for plain export_archive usecase.

Copy link
Contributor

Choose a reason for hiding this comment

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

As you wish, but I think this would surprise me as a user.

constraints=EnsureStr() | EnsureNone()),
no_annex=Parameter(
args=("--no-annex",),
Expand Down Expand Up @@ -230,7 +230,11 @@ def __call__(dataset, filename=None, missing_content='error', no_annex=False,
raise RuntimeError(
"Paranoid authors of DataLad refuse to proceed in a dirty repository"
)
lgr.info("Exporting current tree as an archive since figshare does not support directories")
lgr.info(
"Exporting current tree as an archive since figshare "
"does not support directories")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this message mention the destination?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, adding, thanks

if filename is None:
filename = dataset.path
archive_out = next(
export_archive(
dataset,
Expand Down
5 changes: 5 additions & 0 deletions datalad/plugin/tests/test_export_archive.py
View file
Open in desktop
Expand Up @@ -110,3 +110,8 @@ def test_zip_archive(path):
time.sleep(1.1)
ds.export_archive(filename='my', archivetype='zip')
assert_equal(md5sum('my.zip'), custom1_md5)

# should be able to export without us cd'ing to that ds directory
ds.export_archive(filename=ds.path, archivetype='zip')
default_name = 'datalad_{}.zip'.format(ds.id)
assert_true(os.path.exists(os.path.join(ds.path, default_name)))