Repository: glotzerlab/signac
Pull request #926 (2023-05-29 16:03:11)

Check up on restrictions to signac view
<!-- Provide a general summary of your changes in the title above. -->

## Description
<!-- Describe your changes in detail. -->
<!-- Please indicate if the changes may break existing functionality. -->
I had a space in my state point and was surprised that I couldn't make a signac view. This fix worked for me on mac but I'll run the tests on Windows and Linux through GitHub.


## Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->

The PR that created this restriction was initially a fix involving path separation characters but there was [no reason given for also excluding spaces and asterisks](https://github.com/glotzerlab/signac/pull/233#issuecomment-547587695). I was able to create views in tests with these characters on mac.

I tested this locally with my project that had spaces in the state point after this change, and it worked.

There are [other characters we might want to exclude as well](https://www.informit.com/articles/article.aspx?p=1144082&seqNum=5), like `?`, `[]`, `{}`. These characters might be annoying to escape on the terminal, but a strong use of view is to browse in a file browser.

Perhaps we can emit a warning about special characters being used?

I think if the tests pass in creating views, then we should be good to create views with spaces and asterisks in the state point.

## Checklist:
<!-- This checklist must be complete before merging the pull request. -->
<!-- If you are unsure about any of these items, do not hesitate to ask! -->
- [x] I am familiar with the [Contributing Guidelines](https://github.com/glotzerlab/signac/blob/main/CONTRIBUTING.md).
- [x] I agree with the terms of the [Contributor Agreement](https://github.com/glotzerlab/signac/blob/main/ContributorAgreement.md).
- [x] My name is on the [list of contributors](https://github.com/glotzerlab/signac/blob/main/contributors.yaml).
- [x] The changes introduced by this pull request are covered by existing or newly introduced tests.
- [ ] The [package documentation](https://github.com/glotzerlab/signac/tree/main/doc) and [framework documentation](https://docs.signac.io/) in [signac-docs](https://github.com/glotzerlab/signac-docs) are up to date with these changes.
- [ ] I have updated the [changelog](https://github.com/glotzerlab/signac/blob/main/changelog.txt) and added any related issue and pull request numbers for future reference.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/glotzerlab/signac/pull/926?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) Report
> Merging [#926](https://app.codecov.io/gh/glotzerlab/signac/pull/926?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (3fd88e5) into [main](https://app.codecov.io/gh/glotzerlab/signac/commit/4d5f8a57bd25430c00364d9556b1de7be1ace91b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (4d5f8a5) will **decrease** coverage by `0.06%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #926      +/-   ##
==========================================
- Coverage   85.68%   85.62%   -0.06%     
==========================================
  Files          20       20              
  Lines        3458     3458              
  Branches      758      758              
==========================================
- Hits         2963     2961       -2     
- Misses        335      336       +1     
- Partials      160      161       +1     
```


| [Impacted Files](https://app.codecov.io/gh/glotzerlab/signac/pull/926?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) | Coverage Δ | |
|---|---|---|
| [signac/linked\_view.py](https://app.codecov.io/gh/glotzerlab/signac/pull/926?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2xpbmtlZF92aWV3LnB5) | `88.00% <100.00%> (-1.60%)` | :arrow_down: |


vyasr: Spaces were probably excluded because they don't work on older filesystems, and I'm guessing that one of the supercomputers we originally cared about (maybe the original Stampede, not even Stampede2? Not sure) was an example. I'm fine with removing the restriction and leaving the onus on the user to recognize why a view fails if they have such a filesystem.
bdice: @cbkerr @b-butler Can you push this through?

Patch:
diff --git a/changelog.txt b/changelog.txt
index 34db5be5..e5b1f2ad 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -7,6 +7,14 @@ The **signac** package follows `semantic versioning <https://semver.org/>`_.
 Version 2
 =========
 
+[2.2.0] -- 2023-xx-xx
+---------------------
+
+Changed
++++++++
+
+ - linked views now can contain spaces and other characters except directory separators (#926).
+
 [2.1.0] -- 2023-07-12
 ---------------------
 
diff --git a/signac/linked_view.py b/signac/linked_view.py
index b4d4a725..ab273a72 100644
--- a/signac/linked_view.py
+++ b/signac/linked_view.py
@@ -41,7 +41,7 @@ def create_linked_view(project, prefix=None, job_ids=None, path=None):
         Linked views cannot be created on Windows because
         symbolic links are not supported by the platform.
     RuntimeError
-        When state points contain one of ``[os.sep, " ", "*"]``.
+        When state points contain ``os.sep``.
 
     """
     from .import_export import _check_directory_structure_validity, _make_path_function
@@ -64,19 +64,13 @@ def create_linked_view(project, prefix=None, job_ids=None, path=None):
     key_list = [k for job in jobs for k in job.statepoint().keys()]
     value_list = [v for job in jobs for v in job.statepoint().values()]
     item_list = key_list + value_list
-    bad_chars = [os.sep, " ", "*"]
-    bad_items = [
-        item
-        for item in item_list
-        for char in bad_chars
-        if isinstance(item, str) and char in item
-    ]
+    bad_items = [item for item in item_list if isinstance(item, str) and os.sep in item]
 
     if any(bad_items):
         err_msg = " ".join(
             [
-                f"In order to use view, state points should not contain {bad_chars}:",
-                *bad_items,
+                f"In order to use view, state points should not contain {os.sep}:",
+                str(set(bad_items)),
             ]
         )
         raise RuntimeError(err_msg)


Test patch:
diff --git a/tests/test_project.py b/tests/test_project.py
index e268c4f9..565a2d5b 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -2038,13 +2038,22 @@ class TestLinkedViewProject(TestProjectBase):
 
     @pytest.mark.skipif(WINDOWS, reason="Linked views unsupported on Windows.")
     def test_create_linked_view_with_slash_raises_error(self):
-        bad_chars = [os.sep, " ", "*"]
-        statepoints = [{f"a{i}b": 0, "b": f"bad{i}val"} for i in bad_chars]
+        statepoint = {"b": f"bad{os.sep}val"}
+        view_prefix = os.path.join(self._tmp_pr, "view")
+        self.project.open_job(statepoint).init()
+        with pytest.raises(RuntimeError):
+            self.project.create_linked_view(prefix=view_prefix)
+
+    @pytest.mark.skipif(WINDOWS, reason="Linked views unsupported on Windows.")
+    def test_create_linked_view_weird_chars_in_file_name(self):
+        shell_escaped_chars = [" ", "*", "~"]
+        statepoints = [
+            {f"a{i}b": 0, "b": f"escaped{i}val"} for i in shell_escaped_chars
+        ]
         view_prefix = os.path.join(self._tmp_pr, "view")
         for sp in statepoints:
             self.project.open_job(sp).init()
-            with pytest.raises(RuntimeError):
-                self.project.create_linked_view(prefix=view_prefix)
+            self.project.create_linked_view(prefix=view_prefix)
 
     @pytest.mark.skipif(WINDOWS, reason="Linked views unsupported on Windows.")
     def test_create_linked_view_duplicate_paths(self):