From 0c37f2718f4dbd65a4cd7ef9767f0efe8b399d8f Mon Sep 17 00:00:00 2001
From: Mohammad Alisafaee <mohammad.alisafaee@epfl.ch>
Date: Mon, 6 Jan 2020 17:17:15 +0100
Subject: [PATCH 1/3] fix: create datasets with similar titles

---
 renku/cli/dataset.py                | 12 +++----
 renku/core/commands/dataset.py      |  8 ++---
 renku/core/management/datasets.py   | 16 ++++-----
 tests/cli/test_datasets.py          | 52 +++++++++++------------------
 tests/core/commands/test_dataset.py |  8 ++++-
 5 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/renku/cli/dataset.py b/renku/cli/dataset.py
index ab9564d5e0..279199707c 100644
--- a/renku/cli/dataset.py
+++ b/renku/cli/dataset.py
@@ -317,9 +317,7 @@ def dataset(ctx, revision, datadir, format):
 
 @dataset.command()
 @click.argument('name')
-@click.option(
-    '--short-name', default='', help='A convenient name for dataset.'
-)
+@click.option('--title', default='', help='Title of the dataset.')
 @click.option(
     '-d', '--description', default='', help='Dataset\'s description.'
 )
@@ -330,13 +328,13 @@ def dataset(ctx, revision, datadir, format):
     multiple=True,
     help='Creator\'s name and email ("Name <email>").'
 )
-def create(name, short_name, description, creator):
+def create(name, title, description, creator):
     """Create an empty dataset in the current repo."""
     creators = creator or ()
 
     new_dataset = create_dataset(
-        name=name,
-        short_name=short_name,
+        short_name=name,
+        title=title,
         description=description,
         creators=creators
     )
@@ -565,7 +563,7 @@ def export_(id, provider, publish, tag):
 @dataset.command('import')
 @click.argument('uri')
 @click.option(
-    '--short-name', default='', help='A convenient name for dataset.'
+    '--short-name', default=None, help='A convenient name for dataset.'
 )
 @click.option(
     '-x',
diff --git a/renku/core/commands/dataset.py b/renku/core/commands/dataset.py
index 0033c671ef..27f45601f7 100644
--- a/renku/core/commands/dataset.py
+++ b/renku/core/commands/dataset.py
@@ -69,11 +69,11 @@ def dataset_parent(client, revision, datadir, format, ctx=None):
 )
 def create_dataset(
     client,
-    name,
-    short_name=None,
+    short_name,
+    title=None,
     description=None,
     creators=None,
-    commit_message=None,
+    commit_message=None
 ):
     """Create an empty dataset in the current repo.
 
@@ -89,8 +89,8 @@ def create_dataset(
         creators = [Person.from_dict(creator) for creator in creators]
 
     dataset, _, __ = client.create_dataset(
-        name=name,
         short_name=short_name,
+        title=title,
         description=description,
         creators=creators
     )
diff --git a/renku/core/management/datasets.py b/renku/core/management/datasets.py
index 14bea5a8e4..6d52651923 100644
--- a/renku/core/management/datasets.py
+++ b/renku/core/management/datasets.py
@@ -40,7 +40,7 @@
 from renku.core.management.clone import clone
 from renku.core.management.config import RENKU_HOME
 from renku.core.models.datasets import Dataset, DatasetFile, DatasetTag, \
-    generate_default_short_name, is_dataset_name_valid
+    is_dataset_name_valid
 from renku.core.models.git import GitURL
 from renku.core.models.locals import with_reference
 from renku.core.models.provenance.agents import Person
@@ -131,7 +131,7 @@ def with_dataset(self, short_name=None, create=False):
 
             clean_up_required = True
             dataset, path, dataset_ref = self.create_dataset(
-                name=short_name, short_name=short_name
+                short_name=short_name
             )
         elif create:
             raise errors.DatasetExistsError(
@@ -154,14 +154,11 @@ def with_dataset(self, short_name=None, create=False):
         dataset.to_yaml()
 
     def create_dataset(
-        self, name, short_name=None, description='', creators=None
+        self, short_name=None, title=None, description=None, creators=None
     ):
         """Create a dataset."""
-        if not name:
-            raise errors.ParameterError('Dataset name must be provided.')
-
         if not short_name:
-            short_name = generate_default_short_name(name, None)
+            raise errors.ParameterError('Dataset name must be provided.')
 
         if not is_dataset_name_valid(short_name):
             raise errors.ParameterError(
@@ -173,6 +170,9 @@ def create_dataset(
                 'Dataset exists: "{}".'.format(short_name)
             )
 
+        if not title:
+            title = short_name
+
         identifier = str(uuid.uuid4())
 
         path = self.renku_datasets_path / identifier / self.METADATA
@@ -191,8 +191,8 @@ def create_dataset(
             dataset = Dataset(
                 client=self,
                 identifier=identifier,
-                name=name,
                 short_name=short_name,
+                name=title,
                 description=description,
                 creator=creators
             )
diff --git a/tests/cli/test_datasets.py b/tests/cli/test_datasets.py
index d3460d2367..c0c0701be5 100644
--- a/tests/cli/test_datasets.py
+++ b/tests/cli/test_datasets.py
@@ -61,7 +61,7 @@ def test_datasets_create_with_metadata(runner, client):
     """Test creating a dataset with metadata."""
     result = runner.invoke(
         cli, [
-            'dataset', 'create', 'my-dataset', '--short-name', 'short-name',
+            'dataset', 'create', 'my-dataset', '--title', 'Long Title',
             '--description', 'some description here', '-c',
             'John Doe <john.doe@mail.ch>', '-c',
             'John Smiths<john.smiths@mail.ch>'
@@ -70,9 +70,9 @@ def test_datasets_create_with_metadata(runner, client):
     assert 0 == result.exit_code
     assert 'OK' in result.output
 
-    dataset = client.load_dataset(name='short-name')
-    assert dataset.name == 'my-dataset'
-    assert dataset.short_name == 'short-name'
+    dataset = client.load_dataset(name='my-dataset')
+    assert dataset.name == 'Long Title'
+    assert dataset.short_name == 'my-dataset'
     assert dataset.description == 'some description here'
     assert 'John Doe' in [c.name for c in dataset.creator]
     assert 'john.doe@mail.ch' in [c.email for c in dataset.creator]
@@ -80,16 +80,16 @@ def test_datasets_create_with_metadata(runner, client):
     assert 'john.smiths@mail.ch' in [c.email for c in dataset.creator]
 
 
-def test_datasets_create_different_short_names(runner, client):
-    """Test creating datasets with same name but different short_name."""
+def test_datasets_create_different_names(runner, client):
+    """Test creating datasets with same title but different short_name."""
     result = runner.invoke(
-        cli, ['dataset', 'create', 'dataset', '--short-name', 'dataset-1']
+        cli, ['dataset', 'create', 'dataset-1', '--title', 'title']
     )
     assert 0 == result.exit_code
     assert 'OK' in result.output
 
     result = runner.invoke(
-        cli, ['dataset', 'create', 'dataset', '--short-name', 'dataset-2']
+        cli, ['dataset', 'create', 'dataset-2', '--title', 'title']
     )
     assert 0 == result.exit_code
     assert 'OK' in result.output
@@ -107,23 +107,16 @@ def test_datasets_create_with_same_name(runner, client):
 
 
 @pytest.mark.parametrize(
-    'name,short_name',
-    [('v.a.l.i.d_internal-name', 'v.a.l.i.d_internal-name'),
-     ('any name /@#$!', 'any_name'),
-     ('name longer than 24 characters', 'name_longer_than_24_char'),
-     ('semi valid-name', 'semi_validname'), ('dataset/new', 'datasetnew'),
-     ('/dataset', 'dataset'), ('dataset/', 'dataset')]
+    'name', [
+        'any name /@#$!', 'name longer than 24 characters', 'semi valid-name',
+        'dataset/new', '/dataset', 'dataset/'
+    ]
 )
-def test_datasets_valid_name(runner, client, name, short_name):
-    """Test creating datasets with valid name."""
+def test_datasets_invalid_name(runner, client, name):
+    """Test creating datasets with invalid name."""
     result = runner.invoke(cli, ['dataset', 'create', name])
-    assert 0 == result.exit_code
-    assert 'Use the name "{}"'.format(short_name) in result.output
-    assert 'OK' in result.output
-
-    dataset = client.load_dataset(name=short_name)
-    assert dataset.name == name
-    assert dataset.short_name == short_name
+    assert 2 == result.exit_code
+    assert 'Dataset name "{}" is not valid.'.format(name) in result.output
 
 
 def test_datasets_create_dirty(runner, project, client):
@@ -739,29 +732,24 @@ def test_datasets_ls_files_correct_paths(tmpdir, runner, project):
         assert Path(record['path']).exists()
 
 
-def test_datasets_ls_files_with_display_name(directory_tree, runner, project):
+def test_datasets_ls_files_with_name(directory_tree, runner, project):
     """Test listing of data within dataset with include/exclude filters."""
     # create a dataset
     result = runner.invoke(
-        cli, ['dataset', 'create', 'my-dataset', '--short-name', 'short-name']
+        cli, ['dataset', 'create', 'my-dataset', '--title', 'Long Title']
     )
     assert 0 == result.exit_code
 
     # add data to dataset
     result = runner.invoke(
         cli,
-        ['dataset', 'add', 'short-name', directory_tree.strpath],
+        ['dataset', 'add', 'my-dataset', directory_tree.strpath],
         catch_exceptions=False,
     )
     assert 0 == result.exit_code
 
-    # list files with dataset name
-    result = runner.invoke(cli, ['dataset', 'ls-files', 'my-dataset'])
-    assert 0 == result.exit_code
-    assert 'dir2/file2' not in result.output
-
     # list files with short_name
-    result = runner.invoke(cli, ['dataset', 'ls-files', 'short-name'])
+    result = runner.invoke(cli, ['dataset', 'ls-files', 'my-dataset'])
     assert 0 == result.exit_code
     assert 'dir2/file2' in result.output
 
diff --git a/tests/core/commands/test_dataset.py b/tests/core/commands/test_dataset.py
index a900e3b3d1..baa5a251b3 100644
--- a/tests/core/commands/test_dataset.py
+++ b/tests/core/commands/test_dataset.py
@@ -187,7 +187,13 @@ def test_dataset_serialization(dataset):
 
 def test_create_dataset_custom_message(project):
     """Test create dataset custom message."""
-    create_dataset('ds1', commit_message='my awesome dataset')
+    create_dataset(
+        'ds1',
+        title='',
+        description='',
+        creators=[],
+        commit_message='my awesome dataset'
+    )
 
     last_commit = Repo('.').head.commit
     assert 'my awesome dataset' == last_commit.message

From 7550f9e1a519471a949091d812a79d620d37eba8 Mon Sep 17 00:00:00 2001
From: Mohammad Alisafaee <mohammad.alisafaee@epfl.ch>
Date: Mon, 13 Jan 2020 11:15:50 +0100
Subject: [PATCH 2/3] chore: rename name to short_name

---
 renku/cli/dataset.py              | 56 +++++++++++++++----------------
 renku/core/commands/dataset.py    | 54 +++++++++++++++--------------
 renku/core/management/datasets.py | 28 ++++++++--------
 renku/core/models/datasets.py     | 15 +++++----
 tests/cli/test_datasets.py        | 16 ++++-----
 5 files changed, 86 insertions(+), 83 deletions(-)

diff --git a/renku/cli/dataset.py b/renku/cli/dataset.py
index 279199707c..e2620114ca 100644
--- a/renku/cli/dataset.py
+++ b/renku/cli/dataset.py
@@ -316,7 +316,7 @@ def dataset(ctx, revision, datadir, format):
 
 
 @dataset.command()
-@click.argument('name')
+@click.argument('short_name')
 @click.option('--title', default='', help='Title of the dataset.')
 @click.option(
     '-d', '--description', default='', help='Dataset\'s description.'
@@ -328,12 +328,12 @@ def dataset(ctx, revision, datadir, format):
     multiple=True,
     help='Creator\'s name and email ("Name <email>").'
 )
-def create(name, title, description, creator):
+def create(short_name, title, description, creator):
     """Create an empty dataset in the current repo."""
     creators = creator or ()
 
     new_dataset = create_dataset(
-        short_name=name,
+        short_name=short_name,
         title=title,
         description=description,
         creators=creators
@@ -359,7 +359,7 @@ def edit(dataset_id):
 
 
 @dataset.command()
-@click.argument('name')
+@click.argument('short_name')
 @click.argument('urls', nargs=-1)
 @click.option('--link', is_flag=True, help='Creates a hard link.')
 @click.option(
@@ -391,12 +391,12 @@ def edit(dataset_id):
 @click.option(
     '--ref', default=None, help='Add files from a specific commit/tag/branch.'
 )
-def add(name, urls, link, force, create, sources, destination, ref):
+def add(short_name, urls, link, force, create, sources, destination, ref):
     """Add data to a dataset."""
     progress = partial(progressbar, label='Adding data to dataset')
     add_file(
         urls=urls,
-        name=name,
+        short_name=short_name,
         link=link,
         force=force,
         create=create,
@@ -408,7 +408,7 @@ def add(name, urls, link, force, create, sources, destination, ref):
 
 
 @dataset.command('ls-files')
-@click.argument('names', nargs=-1)
+@click.argument('short_names', nargs=-1)
 @click.option(
     '--creators',
     help='Filter files which where authored by specific creators. '
@@ -434,13 +434,13 @@ def add(name, urls, link, force, create, sources, destination, ref):
     default='tabular',
     help='Choose an output format.'
 )
-def ls_files(names, creators, include, exclude, format):
+def ls_files(short_names, creators, include, exclude, format):
     """List files in dataset."""
-    echo_via_pager(list_files(names, creators, include, exclude, format))
+    echo_via_pager(list_files(short_names, creators, include, exclude, format))
 
 
 @dataset.command()
-@click.argument('name')
+@click.argument('short_name')
 @click.option(
     '-I',
     '--include',
@@ -456,13 +456,13 @@ def ls_files(names, creators, include, exclude, format):
 @click.option(
     '-y', '--yes', is_flag=True, help='Confirm unlinking of all files.'
 )
-def unlink(name, include, exclude, yes):
+def unlink(short_name, include, exclude, yes):
     """Remove matching files from a dataset."""
-    with file_unlink(name, include, exclude) as records:
+    with file_unlink(short_name, include, exclude) as records:
         if not yes and records:
             prompt_text = (
                 'You are about to remove '
-                'following from "{0}" dataset.\n'.format(dataset.name) +
+                'following from "{0}" dataset.\n'.format(short_name) +
                 '\n'.join([str(record.full_path) for record in records]) +
                 '\nDo you wish to continue?'
             )
@@ -472,8 +472,8 @@ def unlink(name, include, exclude, yes):
 
 
 @dataset.command('rm')
-@click.argument('names', nargs=-1)
-def remove(names):
+@click.argument('short_names', nargs=-1)
+def remove(short_names):
     """Delete a dataset."""
     datasetscontext = partial(
         progressbar,
@@ -486,7 +486,7 @@ def remove(names):
         item_show_func=lambda item: item.name if item else '',
     )
     dataset_remove(
-        names,
+        short_names,
         with_output=True,
         datasetscontext=datasetscontext,
         referencescontext=referencescontext
@@ -495,38 +495,38 @@ def remove(names):
 
 
 @dataset.command('tag')
-@click.argument('name')
+@click.argument('short_name')
 @click.argument('tag')
 @click.option(
     '-d', '--description', default='', help='A description for this tag'
 )
 @click.option('--force', is_flag=True, help='Allow overwriting existing tags.')
-def tag(name, tag, description, force):
+def tag(short_name, tag, description, force):
     """Create a tag for a dataset."""
-    tag_dataset_with_client(name, tag, description, force)
+    tag_dataset_with_client(short_name, tag, description, force)
     click.secho('OK', fg='green')
 
 
 @dataset.command('rm-tags')
-@click.argument('name')
+@click.argument('short_name')
 @click.argument('tags', nargs=-1)
-def remove_tags(name, tags):
+def remove_tags(short_name, tags):
     """Remove tags from a dataset."""
-    remove_dataset_tags(name, tags)
+    remove_dataset_tags(short_name, tags)
     click.secho('OK', fg='green')
 
 
 @dataset.command('ls-tags')
-@click.argument('name')
+@click.argument('short_name')
 @click.option(
     '--format',
     type=click.Choice(DATASET_TAGS_FORMATS),
     default='tabular',
     help='Choose an output format.'
 )
-def ls_tags(name, format):
+def ls_tags(short_name, format):
     """List all tags of a dataset."""
-    tags_output = list_tags(name, format)
+    tags_output = list_tags(short_name, format)
     click.echo(tags_output)
 
 
@@ -610,7 +610,7 @@ def finalize(self):
 
 
 @dataset.command('update')
-@click.argument('names', nargs=-1)
+@click.argument('short_names', nargs=-1)
 @click.option(
     '--creators',
     help='Filter files which where authored by specific creators. '
@@ -638,11 +638,11 @@ def finalize(self):
     is_flag=True,
     help='Delete local files that are deleted from remote.'
 )
-def update(names, creators, include, exclude, ref, delete):
+def update(short_names, creators, include, exclude, ref, delete):
     """Updates files in dataset from a remote Git repo."""
     progress_context = partial(progressbar, label='Updating files')
     update_datasets(
-        names=names,
+        short_names=short_names,
         creators=creators,
         include=include,
         exclude=exclude,
diff --git a/renku/core/commands/dataset.py b/renku/core/commands/dataset.py
index 27f45601f7..b697613653 100644
--- a/renku/core/commands/dataset.py
+++ b/renku/core/commands/dataset.py
@@ -71,7 +71,7 @@ def create_dataset(
     client,
     short_name,
     title=None,
-    description=None,
+    description='',
     creators=None,
     commit_message=None
 ):
@@ -124,7 +124,7 @@ def edit_dataset(client, dataset_id, transform_fn, commit_message=None):
 def add_file(
     client,
     urls,
-    name,
+    short_name,
     link=False,
     force=False,
     create=False,
@@ -139,7 +139,7 @@ def add_file(
     add_to_dataset(
         client=client,
         urls=urls,
-        short_name=name,
+        short_name=short_name,
         link=link,
         force=force,
         create=create,
@@ -219,11 +219,11 @@ def add_to_dataset(
 
 
 @pass_local_client(clean=False, commit=False)
-def list_files(client, names, creators, include, exclude, format):
+def list_files(client, short_names, creators, include, exclude, format):
     """List files in dataset."""
     records = _filter(
         client,
-        names=names,
+        short_names=short_names,
         creators=creators,
         include=include,
         exclude=exclude
@@ -238,15 +238,15 @@ def list_files(client, names, creators, include, exclude, format):
     commit_only=COMMIT_DIFF_STRATEGY,
 )
 @contextmanager
-def file_unlink(client, name, include, exclude, commit_message=None):
+def file_unlink(client, short_name, include, exclude, commit_message=None):
     """Remove matching files from a dataset."""
-    dataset = client.load_dataset(name=name)
+    dataset = client.load_dataset(short_name=short_name)
 
     if not dataset:
         raise ParameterError('Dataset does not exist.')
 
     records = _filter(
-        client, names=[dataset.name], include=include, exclude=exclude
+        client, short_names=[short_name], include=include, exclude=exclude
     )
     if not records:
         raise ParameterError('No records found.')
@@ -266,18 +266,18 @@ def file_unlink(client, name, include, exclude, commit_message=None):
 )
 def dataset_remove(
     client,
-    names,
+    short_names,
     with_output=False,
     datasetscontext=contextlib.nullcontext,
     referencescontext=contextlib.nullcontext,
     commit_message=None
 ):
     """Delete a dataset."""
-    datasets = {name: client.get_dataset_path(name) for name in names}
+    datasets = {name: client.get_dataset_path(name) for name in short_names}
 
     if not datasets:
         raise ParameterError(
-            'use dataset name or identifier', param_hint='names'
+            'use dataset short_name or identifier', param_hint='short_names'
         )
 
     unknown = [
@@ -286,7 +286,7 @@ def dataset_remove(
     ]
     if unknown:
         raise ParameterError(
-            'unknown datasets ' + ', '.join(unknown), param_hint='names'
+            'unknown datasets ' + ', '.join(unknown), param_hint='short_names'
         )
 
     datasets = set(datasets.values())
@@ -496,7 +496,7 @@ def import_dataset(
 )
 def update_datasets(
     client,
-    names,
+    short_names,
     creators,
     include,
     exclude,
@@ -508,7 +508,7 @@ def update_datasets(
     """Update files from a remote Git repo."""
     records = _filter(
         client,
-        names=names,
+        short_names=short_names,
         creators=creators,
         include=include,
         exclude=exclude
@@ -527,7 +527,7 @@ def update_datasets(
             dataset = datasets.get(dataset_name)
 
             if not dataset:
-                dataset = client.load_dataset(name=dataset_name)
+                dataset = client.load_dataset(short_name=dataset_name)
                 datasets[dataset_name] = dataset
 
             file_.dataset = dataset
@@ -576,10 +576,12 @@ def _include_exclude(file_path, include=None, exclude=None):
     return True
 
 
-def _filter(client, names=None, creators=None, include=None, exclude=None):
+def _filter(
+    client, short_names=None, creators=None, include=None, exclude=None
+):
     """Filter dataset files by specified filters.
 
-    :param names: Filter by specified dataset names.
+    :param short_names: Filter by specified dataset short_names.
     :param creators: Filter by creators.
     :param include: Include files matching file pattern.
     :param exclude: Exclude files matching file pattern.
@@ -592,7 +594,7 @@ def _filter(client, names=None, creators=None, include=None, exclude=None):
 
     records = []
     for path_, dataset in client.datasets.items():
-        if not names or dataset.short_name in names:
+        if not short_names or dataset.short_name in short_names:
             for file_ in dataset.files:
                 file_.dataset = dataset.name
                 path_ = file_.full_path.relative_to(client.path)
@@ -616,15 +618,15 @@ def _filter(client, names=None, creators=None, include=None, exclude=None):
     commit_only=COMMIT_DIFF_STRATEGY,
 )
 def tag_dataset_with_client(
-    client, name, tag, description, force=False, commit_message=None
+    client, short_name, tag, description, force=False, commit_message=None
 ):
     """Creates a new tag for a dataset and injects a LocalClient."""
-    tag_dataset(client, name, tag, description, force)
+    tag_dataset(client, short_name, tag, description, force)
 
 
-def tag_dataset(client, name, tag, description, force=False):
+def tag_dataset(client, short_name, tag, description, force=False):
     """Creates a new tag for a dataset."""
-    dataset_ = client.load_dataset(name)
+    dataset_ = client.load_dataset(short_name)
     if not dataset_:
         raise ParameterError('Dataset not found.')
 
@@ -641,9 +643,9 @@ def tag_dataset(client, name, tag, description, force=False):
     commit=True,
     commit_only=COMMIT_DIFF_STRATEGY,
 )
-def remove_dataset_tags(client, name, tags, commit_message=True):
+def remove_dataset_tags(client, short_name, tags, commit_message=True):
     """Removes tags from a dataset."""
-    dataset = client.load_dataset(name)
+    dataset = client.load_dataset(short_name)
     if not dataset:
         raise ParameterError('Dataset not found.')
 
@@ -656,9 +658,9 @@ def remove_dataset_tags(client, name, tags, commit_message=True):
 
 
 @pass_local_client(clean=False, commit=False)
-def list_tags(client, name, format):
+def list_tags(client, short_name, format):
     """List all tags for a dataset."""
-    dataset_ = client.load_dataset(name)
+    dataset_ = client.load_dataset(short_name)
 
     if not dataset_:
         raise ParameterError('Dataset not found.')
diff --git a/renku/core/management/datasets.py b/renku/core/management/datasets.py
index 6d52651923..68457462ac 100644
--- a/renku/core/management/datasets.py
+++ b/renku/core/management/datasets.py
@@ -40,7 +40,7 @@
 from renku.core.management.clone import clone
 from renku.core.management.config import RENKU_HOME
 from renku.core.models.datasets import Dataset, DatasetFile, DatasetTag, \
-    is_dataset_name_valid
+    is_dataset_short_name_valid
 from renku.core.models.git import GitURL
 from renku.core.models.locals import with_reference
 from renku.core.models.provenance.agents import Person
@@ -99,30 +99,30 @@ def load_dataset_from_path(self, path, commit=None):
             path = self.path / path
         return Dataset.from_yaml(path, client=self, commit=commit)
 
-    def get_dataset_path(self, name):
-        """Get dataset path from name."""
-        path = self.renku_datasets_path / name / self.METADATA
+    def get_dataset_path(self, short_name):
+        """Get dataset path from short_name."""
+        path = self.renku_datasets_path / short_name / self.METADATA
         if not path.exists():
             try:
                 path = LinkReference(
-                    client=self, name='datasets/' + name
+                    client=self, name='datasets/' + short_name
                 ).reference
             except errors.ParameterError:
                 return None
 
         return path
 
-    def load_dataset(self, name=None):
+    def load_dataset(self, short_name=None):
         """Load dataset reference file."""
-        if name:
-            path = self.get_dataset_path(name)
+        if short_name:
+            path = self.get_dataset_path(short_name)
             if path and path.exists():
                 return self.load_dataset_from_path(path)
 
     @contextmanager
     def with_dataset(self, short_name=None, create=False):
         """Yield an editable metadata object for a dataset."""
-        dataset = self.load_dataset(name=short_name)
+        dataset = self.load_dataset(short_name=short_name)
         clean_up_required = False
 
         if dataset is None:
@@ -154,18 +154,18 @@ def with_dataset(self, short_name=None, create=False):
         dataset.to_yaml()
 
     def create_dataset(
-        self, short_name=None, title=None, description=None, creators=None
+        self, short_name=None, title=None, description='', creators=None
     ):
         """Create a dataset."""
         if not short_name:
-            raise errors.ParameterError('Dataset name must be provided.')
+            raise errors.ParameterError('Dataset short_name must be provided.')
 
-        if not is_dataset_name_valid(short_name):
+        if not is_dataset_short_name_valid(short_name):
             raise errors.ParameterError(
-                'Dataset name "{}" is not valid.'.format(short_name)
+                'Dataset short_name "{}" is not valid.'.format(short_name)
             )
 
-        if self.load_dataset(name=short_name):
+        if self.load_dataset(short_name=short_name):
             raise errors.DatasetExistsError(
                 'Dataset exists: "{}".'.format(short_name)
             )
diff --git a/renku/core/models/datasets.py b/renku/core/models/datasets.py
index c0610a69bc..cd6e497d66 100644
--- a/renku/core/models/datasets.py
+++ b/renku/core/models/datasets.py
@@ -364,7 +364,7 @@ def _now(self):
     def short_name_validator(self, attribute, value):
         """Validate short_name."""
         # short_name might have been scaped and have '%' in it
-        if value and not is_dataset_name_valid(value, safe='%'):
+        if value and not is_dataset_short_name_valid(value, safe='%'):
             raise errors.ParameterError(
                 'Invalid "short_name": {}'.format(value)
             )
@@ -540,12 +540,13 @@ def __attrs_post_init__(self):
             )
 
 
-def is_dataset_name_valid(name, safe=''):
-    """A valid name is a valid Git reference name with no /."""
-    # TODO make name an RFC 3986 compatible name and migrate old projects
+def is_dataset_short_name_valid(short_name, safe=''):
+    """A valid short_name is a valid Git reference name with no /."""
+    # TODO make short_name an RFC 3986 compatible and migrate old projects
     return (
-        name and LinkReference.check_ref_format(name, no_slashes=True) and
-        '/' not in name
+        short_name and
+        LinkReference.check_ref_format(short_name, no_slashes=True) and
+        '/' not in short_name
     )
 
 
@@ -553,7 +554,7 @@ def generate_default_short_name(dataset_name, dataset_version):
     """Get dataset short_name."""
     # For compatibility with older versions use name as short_name
     # if it is valid; otherwise, use encoded name
-    if is_dataset_name_valid(dataset_name):
+    if is_dataset_short_name_valid(dataset_name):
         return dataset_name
 
     name = re.sub(r'\s+', ' ', dataset_name)
diff --git a/tests/cli/test_datasets.py b/tests/cli/test_datasets.py
index c0c0701be5..e00604eb7a 100644
--- a/tests/cli/test_datasets.py
+++ b/tests/cli/test_datasets.py
@@ -45,7 +45,7 @@ def test_datasets_create_clean(runner, project, client):
     assert 0 == result.exit_code
     assert 'OK' in result.output
 
-    dataset = client.load_dataset(name='dataset')
+    dataset = client.load_dataset('dataset')
     assert dataset
 
     staged = client.repo.index.diff('HEAD')
@@ -70,7 +70,7 @@ def test_datasets_create_with_metadata(runner, client):
     assert 0 == result.exit_code
     assert 'OK' in result.output
 
-    dataset = client.load_dataset(name='my-dataset')
+    dataset = client.load_dataset('my-dataset')
     assert dataset.name == 'Long Title'
     assert dataset.short_name == 'my-dataset'
     assert dataset.description == 'some description here'
@@ -116,7 +116,7 @@ def test_datasets_invalid_name(runner, client, name):
     """Test creating datasets with invalid name."""
     result = runner.invoke(cli, ['dataset', 'create', name])
     assert 2 == result.exit_code
-    assert 'Dataset name "{}" is not valid.'.format(name) in result.output
+    assert 'short_name "{}" is not valid.'.format(name) in result.output
 
 
 def test_datasets_create_dirty(runner, project, client):
@@ -129,7 +129,7 @@ def test_datasets_create_dirty(runner, project, client):
     assert 0 == result.exit_code
     assert 'OK' in result.output
 
-    dataset = client.load_dataset(name='dataset')
+    dataset = client.load_dataset('dataset')
     assert dataset
 
     staged = client.repo.index.diff('HEAD')
@@ -871,7 +871,7 @@ def test_dataset_rm(tmpdir, runner, project, client):
 
     # check output
     assert 'OK' in result.output
-    assert not client.load_dataset(name='my-dataset')
+    assert not client.load_dataset('my-dataset')
 
     result = runner.invoke(cli, ['doctor'], catch_exceptions=False)
     assert 0 == result.exit_code
@@ -890,7 +890,7 @@ def test_dataset_rm_commit(tmpdir, runner, project, client):
 
     # check output
     assert 'OK' in result.output
-    assert not client.load_dataset(name='my-dataset')
+    assert not client.load_dataset('my-dataset')
 
     # Dirty repository check.
     result = runner.invoke(cli, ['status'])
@@ -914,7 +914,7 @@ def test_dataset_edit(runner, client, project):
     assert 0 == result.exit_code
     assert 'OK' in result.output
 
-    dataset = client.load_dataset(name='dataset')
+    dataset = client.load_dataset('dataset')
 
     result = runner.invoke(
         cli, ['dataset', 'edit', dataset.identifier],
@@ -935,7 +935,7 @@ def test_dataset_edit_dirty(runner, client, project):
     assert 0 == result.exit_code
     assert 'OK' in result.output
 
-    dataset = client.load_dataset(name='dataset')
+    dataset = client.load_dataset('dataset')
 
     result = runner.invoke(
         cli, ['dataset', 'edit', dataset.identifier], input='wq'

From a335614d52244826b1cc0148d99d7ef6bd9bb0bf Mon Sep 17 00:00:00 2001
From: Mohammad Alisafaee <mohammad.alisafaee@epfl.ch>
Date: Wed, 15 Jan 2020 16:18:11 +0100
Subject: [PATCH 3/3] review: apply comments

---
 renku/core/models/datasets.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/renku/core/models/datasets.py b/renku/core/models/datasets.py
index cd6e497d66..237affbeed 100644
--- a/renku/core/models/datasets.py
+++ b/renku/core/models/datasets.py
@@ -364,7 +364,7 @@ def _now(self):
     def short_name_validator(self, attribute, value):
         """Validate short_name."""
         # short_name might have been scaped and have '%' in it
-        if value and not is_dataset_short_name_valid(value, safe='%'):
+        if value and not is_dataset_short_name_valid(value):
             raise errors.ParameterError(
                 'Invalid "short_name": {}'.format(value)
             )
@@ -540,7 +540,7 @@ def __attrs_post_init__(self):
             )
 
 
-def is_dataset_short_name_valid(short_name, safe=''):
+def is_dataset_short_name_valid(short_name):
     """A valid short_name is a valid Git reference name with no /."""
     # TODO make short_name an RFC 3986 compatible and migrate old projects
     return (