Repository: glotzerlab/signac
Pull request #852 (2022-11-06 16:43:33)

Only try to move v1 schema files that exist.
<!-- 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. -->
Makes the final `os.replace` calls in the v1->v2 migration conditional on whether the files exist so that the script doesn't try to move nonexistent files.

## 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. -->
Not every project will have a shell history or a state point cache.

## 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/master/CONTRIBUTING.md).
- [x] I agree with the terms of the [Contributor Agreement](https://github.com/glotzerlab/signac/blob/master/ContributorAgreement.md).
- [x] My name is on the [list of contributors](https://github.com/glotzerlab/signac/blob/master/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/master/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/master/changelog.txt) and added any related issue and pull request numbers for future reference.


codecov[bot]: # [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/852?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) Report
> Merging [#852](https://codecov.io/gh/glotzerlab/signac/pull/852?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (bec55c8) into [master](https://codecov.io/gh/glotzerlab/signac/commit/db2b9fab5d03cfd4ccf27d297c2a0621e8904a48?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (db2b9fa) will **decrease** coverage by `0.46%`.
> The diff coverage is `66.66%`.

```diff
@@            Coverage Diff             @@
##           master     #852      +/-   ##
==========================================
- Coverage   85.75%   85.29%   -0.47%     
==========================================
  Files          51       51              
  Lines        4683     4685       +2     
  Branches      923      924       +1     
==========================================
- Hits         4016     3996      -20     
- Misses        480      496      +16     
- Partials      187      193       +6     
```


| [Impacted Files](https://codecov.io/gh/glotzerlab/signac/pull/852?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) | Coverage Δ | |
|---|---|---|
| [signac/contrib/migration/v1\_to\_v2.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbnRyaWIvbWlncmF0aW9uL3YxX3RvX3YyLnB5) | `88.09% <66.66%> (-1.91%)` | :arrow_down: |
| [signac/core/utility.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvcmUvdXRpbGl0eS5weQ==) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| [signac/contrib/migration/\_\_init\_\_.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbnRyaWIvbWlncmF0aW9uL19faW5pdF9fLnB5) | `89.65% <0.00%> (-3.45%)` | :arrow_down: |
| [signac/core/dict\_manager.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvcmUvZGljdF9tYW5hZ2VyLnB5) | `86.04% <0.00%> (-2.33%)` | :arrow_down: |
| [...ed\_collections/buffers/file\_buffered\_collection.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL19zeW5jZWRfY29sbGVjdGlvbnMvYnVmZmVycy9maWxlX2J1ZmZlcmVkX2NvbGxlY3Rpb24ucHk=) | `95.76% <0.00%> (-1.70%)` | :arrow_down: |
| [...ac/\_synced\_collections/backends/collection\_json.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL19zeW5jZWRfY29sbGVjdGlvbnMvYmFja2VuZHMvY29sbGVjdGlvbl9qc29uLnB5) | `98.33% <0.00%> (-1.67%)` | :arrow_down: |
| [signac/contrib/linked\_view.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbnRyaWIvbGlua2VkX3ZpZXcucHk=) | `88.00% <0.00%> (-1.60%)` | :arrow_down: |
| [signac/\_\_main\_\_.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL19fbWFpbl9fLnB5) | `77.05% <0.00%> (-0.74%)` | :arrow_down: |
| [signac/contrib/job.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbnRyaWIvam9iLnB5) | `89.14% <0.00%> (-0.66%)` | :arrow_down: |
| [signac/contrib/import\_export.py](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbnRyaWIvaW1wb3J0X2V4cG9ydC5weQ==) | `76.98% <0.00%> (-0.27%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/glotzerlab/signac/pull/852/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab)

bdice: I ran into this issue while testing. I'm going to file a more extensive PR with additional fixes soon.

Patch:
diff --git a/changelog.txt b/changelog.txt
index 72d1b49d..d262f58f 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -48,7 +48,7 @@ Removed
  - The ability to pass indexes to various ``Project`` methods (#599).
  - The following ``JobsCursor`` methods: ``groupbydoc``, ``next`` (#601, #604).
  - The ``Project.config`` property is no longer mutable. Use the command line ``$ signac config`` to modify configuration (#608, #246, #244).
- - The following config related functions: ``get_config``, ``search_standard_dirs`` (#674).
+ - The following config related functions: ``get_config``, ``load_config``, ``read_config_file``, ``search_standard_dirs`` (#674, #753, #789, #847).
  - ``Project`` subclasses can no longer define a ``Job`` subclass to use (#588, #693).
  - The ``Collection`` class (#664, #667, #683).
  - The ``project`` CLI subcommand (#752).
diff --git a/signac/common/config.py b/signac/common/config.py
index 10d7be40..daf83777 100644
--- a/signac/common/config.py
+++ b/signac/common/config.py
@@ -6,8 +6,6 @@
 import logging
 import os
 
-from ..common.deprecation import deprecated
-from ..version import __version__
 from .configobj import ConfigObj, ConfigObjError
 from .errors import ConfigError
 from .validate import cfg, get_validator
@@ -114,20 +112,6 @@ def _read_config_file(filename):
     return config
 
 
-@deprecated(
-    deprecated_in="1.8",
-    removed_in="2.0",
-    current_version=__version__,
-    details=(
-        "The read_config_file method is deprecated. Configs should only be "
-        "accessed via a Project instance.",
-    ),
-)
-def read_config_file(filename):
-    """Read a configuration file."""
-    return _read_config_file(filename)
-
-
 def _load_config(path=None):
     """Load configuration from a project directory.
 
@@ -158,20 +142,6 @@ def _load_config(path=None):
     return config
 
 
-@deprecated(
-    deprecated_in="1.8",
-    removed_in="2.0",
-    current_version=__version__,
-    details=(
-        "The load_config method is deprecated. Configs should only be "
-        "accessed via a Project instance.",
-    ),
-)
-def load_config(root=None):
-    """Load configuration, searching upward from a root path."""
-    return _load_config(root)
-
-
 class Config(ConfigObj):
     """Manages configuration for a signac project."""
 
diff --git a/signac/contrib/migration/v1_to_v2.py b/signac/contrib/migration/v1_to_v2.py
index 6f55f616..31b89802 100644
--- a/signac/contrib/migration/v1_to_v2.py
+++ b/signac/contrib/migration/v1_to_v2.py
@@ -85,6 +85,6 @@ def _migrate_v1_to_v2(root_directory):
         ),
     }
     for src, dst in files_to_move.items():
-        os.replace(
-            os.sep.join((root_directory, src)), os.sep.join((root_directory, dst))
-        )
+        src = os.sep.join((root_directory, src))
+        if os.path.isfile(src):
+            os.replace(src, os.sep.join((root_directory, dst)))


Test patch:
diff --git a/tests/test_project.py b/tests/test_project.py
index c2e56cbb..23e64ef8 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -2326,7 +2326,7 @@ class TestProjectSchema(TestProjectBase):
         assert len(migrations) == 0
 
 
-def _initialize_v1_project(dirname, with_workspace=True):
+def _initialize_v1_project(dirname, with_workspace=True, with_other_files=True):
     # Create v1 config file.
     cfg_fn = os.path.join(dirname, "signac.rc")
     workspace_dir = "workspace_dir"
@@ -2345,18 +2345,19 @@ def _initialize_v1_project(dirname, with_workspace=True):
     if with_workspace:
         os.makedirs(os.path.join(dirname, "workspace"))
 
-    # Create a shell history file.
-    history_fn = os.path.join(dirname, ".signac_shell_history")
-    with open(history_fn, "w") as f:
-        f.write("print(project)")
+    if with_other_files:
+        # Create a shell history file.
+        history_fn = os.path.join(dirname, ".signac_shell_history")
+        with open(history_fn, "w") as f:
+            f.write("print(project)")
 
-    # Create a statepoint cache. Note that this cache does not
-    # correspond to actual statepoints since we don't currently have
-    # any in this project, but that's fine for migration testing.
-    sp_cache = os.path.join(dirname, ".signac_sp_cache.json.gz")
-    sp = {"a": 1}
-    with gzip.open(sp_cache, "wb") as f:
-        f.write(json.dumps({calc_id(sp): sp}).encode())
+        # Create a statepoint cache. Note that this cache does not
+        # correspond to actual statepoints since we don't currently have
+        # any in this project, but that's fine for migration testing.
+        sp_cache = os.path.join(dirname, ".signac_sp_cache.json.gz")
+        sp = {"a": 1}
+        with gzip.open(sp_cache, "wb") as f:
+            f.write(json.dumps({calc_id(sp): sp}).encode())
 
     return cfg_fn
 
@@ -2364,11 +2365,14 @@ def _initialize_v1_project(dirname, with_workspace=True):
 class TestSchemaMigration:
     @pytest.mark.parametrize("implicit_version", [True, False])
     @pytest.mark.parametrize("workspace_exists", [True, False])
-    def test_project_schema_version_migration(self, implicit_version, workspace_exists):
+    @pytest.mark.parametrize("with_other_files", [True, False])
+    def test_project_schema_version_migration(
+        self, implicit_version, workspace_exists, with_other_files
+    ):
         from signac.contrib.migration import apply_migrations
 
         with TemporaryDirectory() as dirname:
-            cfg_fn = _initialize_v1_project(dirname, workspace_exists)
+            cfg_fn = _initialize_v1_project(dirname, workspace_exists, with_other_files)
 
             # If no schema version is present in the config it is equivalent to
             # version 0, so we test both explicit and implicit versions.
@@ -2397,8 +2401,11 @@ class TestSchemaMigration:
             assert "0 to 1" in err.getvalue()
             assert "1 to 2" in err.getvalue()
             assert os.path.isfile(project.fn(PROJECT_CONFIG_FN))
-            assert os.path.isfile(project.fn(os.sep.join((".signac", "shell_history"))))
-            assert os.path.isfile(project.fn(Project.FN_CACHE))
+            if with_other_files:
+                assert os.path.isfile(
+                    project.fn(os.sep.join((".signac", "shell_history")))
+                )
+                assert os.path.isfile(project.fn(Project.FN_CACHE))
 
     def test_project_init_old_schema(self):
         with TemporaryDirectory() as dirname: