signac view CLI should not require a prefix

[bdice]

Problem

Specifying a path string to $ signac view requires the user to specify a prefix first, which defaults to view. This results in writing signac view view path which sounds awkward.

I frequently make this mistake and write $ signac view path instead of $ signac view view path (because that sounds silly). @cbkerr also ran across this while writing tests in #642.

Proposed solution

This could be fixed in signac 2.0 by requiring --prefix view to use a prefix besides the default name view.

Originally posted by @bdice in #642 (comment)

CLI help message

$ python -m signac view --help
usage: __main__.py view [-h] [-f FILTER [FILTER ...]]
                        [-d DOC_FILTER [DOC_FILTER ...]]
                        [-j JOB_ID [JOB_ID ...]] [-i INDEX]
                        [prefix] [path]

Generate a human readable set of paths representing state points in the
workspace, e.g.
view/param_name_1/param_value_1/param_name_2/param_value_2/job. The leaf nodes
of this directory structure are symlinks (named "job") into the workspace
directory for that parameter combination. Note that all positional arguments
must be provided before any keyword arguments. In particular, the prefix and
path must be specified before arguments such as the filters, e.g. signac view
$PREFIX $VIEW_PATH -f FILTERS -d DOC_FILTERS.

positional arguments:
  prefix                The path where the view is to be created. Defaults to
                        view.
  path                  The path used for the generation of the linked view
                        hierarchy, defaults to '{{auto}}' (see
                        Project.export_to for information on how this is
                        expanded).

optional arguments:
  -h, --help            show this help message and exit

select:
  -f FILTER [FILTER ...], --filter FILTER [FILTER ...]
                        Limit the view to jobs matching this state point
                        filter.
  -d DOC_FILTER [DOC_FILTER ...], --doc-filter DOC_FILTER [DOC_FILTER ...]
                        Limit the view to jobs matching this document filter.
  -j JOB_ID [JOB_ID ...], --job-id JOB_ID [JOB_ID ...]
                        Limit the view to jobs with these job ids.
  -i INDEX, --index INDEX
                        The filename of an index file.

[cbkerr]

I agree with your proposed solution because I've used that option to have several different views.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[bdice]

I think this might be a good first issue. I believe a solution may only require a couple changes:

signac/signac/__main__.py

Lines 1229 to 1235 in 52ff017

	parser_view.add_argument(
	"prefix",
	type=str,
	nargs="?",
	default="view",
	help="The path where the view is to be created. Defaults to view.",
	)

1. The line "prefix" should be changed to a flag instead of a positional argument, like "--prefix". I think that would make it optional (with the default value of "view" as before).
2. Update tests for the CLI.

• Tests using the default value, like signac view view path should just say signac view path.

signac/tests/test_shell.py

Lines 207 to 211 in 52ff017

	err = self.call(
	"python -m signac view view non_unique".split(),
	error=True,
	raise_error=False,
	)

• Tests using a non-default prefix should be added, and specify --prefix.

[srinathkp]

Hi @bdice , I'm new to signac project. Can I work on this ?

[bdice]

@srinathkp Sure! You'll want to create a fork of this repository and start your work by creating a new branch named feature/cli-view-prefix based on the next branch (which will become signac 2.0). Please feel free to post here if you have questions.

[vyasr]

@srinathkp are you still interested in looking at this? If not, @cbkerr do you want to take a stab at it? We should make this change (on next) prior to 2.0 since it is an API break.

[srinathkp]

No @vyasr . You can assign it to @cbkerr if he's interested.

[kodyt]

Hi all. I started looking at this syntax issue, but I am a little unfamiliar with argparse and signac. We can change the "prefix" to "--prefix" like you mentioned before and add "-p" for shorthand notation to make this an optional argument. @bdice Is this all you meant by making it a flag?

signac/signac/__main__.py

Lines 1229 to 1235 in 52ff017

	parser_view.add_argument(
	"prefix",
	type=str,
	nargs="?",
	default="view",
	help="The path where the view is to be created. Defaults to view.",
	)

Also, I was wondering how I would go about testing this change. Would it be similar to test_view_incomplete_path_spec?

signac/tests/test_shell.py

Lines 196 to 212 in 52ff017

	@pytest.mark.skipif(WINDOWS, reason="Symbolic links are unsupported on Windows.")
	def test_view_incomplete_path_spec(self):
	self.call("python -m signac init".split())
	project = signac.Project()
	sps = [{"a": i} for i in range(3)]
	for sp in sps:
	project.open_job(sp).init()
	os.mkdir("view")
	# An error should be raised if the user-provided path function
	# doesn't make a 1-1 mapping.
	err = self.call(
	"python -m signac view view non_unique".split(),
	error=True,
	raise_error=False,
	)
	assert "duplicate paths" in err

[vyasr]

@kodyt apologies for the slow response, yes your suggestion looks about right! Changing the name of the parameter in add_argument to include -- will make it into a flag instead of a "positional" argument. And the test you pointed out is a reasonable choice for a template. You'll have to change the actual view commands that are called and make sure that the right directories are generated.