From dace571cc88daeb79a81f8bdbf0804fbf863390e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 20 Feb 2022 21:45:26 -0800 Subject: [PATCH 01/24] Remove some unnecessary code in project.py. --- signac/contrib/project.py | 42 ++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/signac/contrib/project.py b/signac/contrib/project.py index d898b862a..b37e2184b 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -260,9 +260,7 @@ def __init__(self, config=None): self._lock = RLock() # Prepare cached properties derived from the project config. - self._id = None - self._rd = None - self._wd = None + _invalidate_config_cache(self) # Ensure that the project id is configured. if self.id is None: @@ -514,18 +512,6 @@ def isfile(self, filename): """ return os.path.isfile(self.fn(filename)) - def _reset_document(self, new_doc): - """Reset document to new document passed. - - Parameters - ---------- - new_doc : dict - The new project document. - - """ - with self._lock: - self.document.reset(new_doc) - @property def document(self): """Get document associated with this project. @@ -554,7 +540,8 @@ def document(self, new_doc): The new project document. """ - self._reset_document(new_doc) + with self._lock: + self.document.reset(new_doc) @property def doc(self): @@ -754,6 +741,12 @@ def _job_dirs(self): ) raise WorkspaceError(error) + @deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + details="The num_jobs method is deprecated. Use len(project) instead.", + ) def num_jobs(self): """Return the number of initialized jobs. @@ -763,6 +756,9 @@ def num_jobs(self): Count of initialized jobs. """ + return len(self) + + def __len__(self): # We simply count the the number of valid directories and avoid building a list # for improved performance. i = 0 @@ -770,8 +766,6 @@ def num_jobs(self): pass return i - __len__ = num_jobs - def _contains_job_id(self, job_id): """Determine whether a job id is in the project's data space. @@ -787,7 +781,7 @@ def _contains_job_id(self, job_id): """ # We can rely on the project workspace to be well-formed, so just use - # str.join with os.sep instead of os.path.join for speed. + # os.sep.join instead of os.path.join for speed. return os.path.exists(os.sep.join((self.workspace(), job_id))) def __contains__(self, job): @@ -2625,6 +2619,12 @@ def __iter__(self): self._project, self._project._find_job_ids(self._filter) ) + @deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + details="Use next(iter(...)) instead.", + ) def next(self): """Return the next element. @@ -2632,10 +2632,6 @@ def next(self): .. deprecated:: 0.9.6 """ - warnings.warn( - "Calling next() directly on a JobsCursor is deprecated! Use next(iter(..)) instead.", - DeprecationWarning, - ) if self._next_iter is None: self._next_iter = iter(self) try: From a530fef2fb4f723b0527beb109cc2feef8702813 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 20 Feb 2022 21:45:41 -0800 Subject: [PATCH 02/24] Make notes on possible deprecations/changes in job.py. --- signac/contrib/job.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index e8315190e..abda18ef9 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -359,6 +359,9 @@ def _statepoint_filename(self): # use str.join with os.sep instead of os.path.join for speed. return os.sep.join((self.workspace(), self.FN_MANIFEST)) + # TODO: Why is ws a property while workspace is a method? Why don't the + # Project methods for workspace/root_directory also behave the same? We + # should standardize this. @property def ws(self): """Alias for :meth:`~Job.workspace`.""" @@ -483,6 +486,11 @@ def statepoint(self): return self._statepoint + # TODO: If we really believe what `reset_statepoint`'s documentation says, + # I think we should deprecate this functionality. If resetting a statepoint + # is a dangerous operation, making it settable via a property is too easy, + # and moreover it's impossible to document effectively because property + # setters don't show up in docs. @statepoint.setter def statepoint(self, new_statepoint): """Assign a new state point to this job. @@ -500,6 +508,7 @@ def sp(self): """Alias for :attr:`~Job.statepoint`.""" return self.statepoint + # TODO: Same concern as statepoint setter. @sp.setter def sp(self, new_sp): """Alias for :attr:`~Job.statepoint`.""" From 102ef00a4f7fe87a499df49bb9202bae5f9d573b Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 20 Feb 2022 22:00:08 -0800 Subject: [PATCH 03/24] Replace all DeprecationWarnings with FutureWarnings. --- signac/contrib/project.py | 16 ++++++++-------- .../backends/collection_json.py | 4 ++-- .../buffers/file_buffered_collection.py | 7 +++---- signac/warnings.py | 2 ++ 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/signac/contrib/project.py b/signac/contrib/project.py index b37e2184b..3a76242ec 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -170,7 +170,7 @@ def find_job_ids(self, filter=None, doc_filter=None): if doc_filter: filter.update(doc_filter) elif doc_filter: - warnings.warn(DOC_FILTER_WARNING, DeprecationWarning) + warnings.warn(DOC_FILTER_WARNING, FutureWarning) filter = doc_filter return self._collection._find(filter) @@ -910,7 +910,7 @@ def detect_schema(self, exclude_const=False, subset=None, index=None): if index is None: index = self.index(include_job_document=False) else: - warnings.warn(INDEX_DEPRECATION_WARNING, DeprecationWarning) + warnings.warn(INDEX_DEPRECATION_WARNING, FutureWarning) if subset is not None: subset = {str(s) for s in subset} index = [doc for doc in index if doc["_id"] in subset] @@ -1022,7 +1022,7 @@ def _find_job_ids(self, filter=None, doc_filter=None, index=None): if index is None: filter = dict(parse_filter(_add_prefix("sp.", filter))) if doc_filter: - warnings.warn(DOC_FILTER_WARNING, DeprecationWarning) + warnings.warn(DOC_FILTER_WARNING, FutureWarning) filter.update(parse_filter(_add_prefix("doc.", doc_filter))) index = self.index(include_job_document=True) elif "doc" in _root_keys(filter): @@ -1030,7 +1030,7 @@ def _find_job_ids(self, filter=None, doc_filter=None, index=None): else: index = self._sp_index() else: - warnings.warn(INDEX_DEPRECATION_WARNING, DeprecationWarning) + warnings.warn(INDEX_DEPRECATION_WARNING, FutureWarning) return Collection(index, _trust=True)._find(filter) @@ -1070,7 +1070,7 @@ def find_jobs(self, filter=None, doc_filter=None): """ filter = dict(parse_filter(_add_prefix("sp.", filter))) if doc_filter: - warnings.warn(DOC_FILTER_WARNING, DeprecationWarning) + warnings.warn(DOC_FILTER_WARNING, FutureWarning) filter.update(parse_filter(_add_prefix("doc.", doc_filter))) return JobsCursor(self, filter) @@ -1512,7 +1512,7 @@ def create_linked_view(self, prefix=None, job_ids=None, index=None, path=None): """ if index is not None: - warnings.warn(INDEX_DEPRECATION_WARNING, DeprecationWarning) + warnings.warn(INDEX_DEPRECATION_WARNING, FutureWarning) from .linked_view import create_linked_view return create_linked_view(self, prefix, job_ids, index, path) @@ -1913,7 +1913,7 @@ def repair(self, fn_statepoints=None, index=None, job_ids=None): if index is not None: for doc in index: self._sp_cache[doc["signac_id"]] = doc["sp"] - warnings.warn(INDEX_DEPRECATION_WARNING, DeprecationWarning) + warnings.warn(INDEX_DEPRECATION_WARNING, FutureWarning) corrupted = [] for job_id in job_ids: try: @@ -2217,7 +2217,7 @@ def create_access_module(self, filename=None, main=True, master=None): """ if master is not None: warnings.warn( - "The parameter master has been renamed to main.", DeprecationWarning + "The parameter master has been renamed to main.", FutureWarning ) main = master diff --git a/signac/synced_collections/backends/collection_json.py b/signac/synced_collections/backends/collection_json.py index ce4caa65b..ca2b8fc06 100644 --- a/signac/synced_collections/backends/collection_json.py +++ b/signac/synced_collections/backends/collection_json.py @@ -49,7 +49,7 @@ def _str_key(key): warnings.warn( f"Use of {type(key).__name__} as key is deprecated " "and will be removed in version 2.0", - DeprecationWarning, + FutureWarning, ) key = str(key) return key @@ -146,7 +146,7 @@ def json_attr_dict_validator(data): warnings.warn( f"Use of {type(key).__name__} as key is deprecated " "and will be removed in version 2.0.", - DeprecationWarning, + FutureWarning, ) data[str(key)] = data.pop(key) else: diff --git a/signac/synced_collections/buffers/file_buffered_collection.py b/signac/synced_collections/buffers/file_buffered_collection.py index e7519f0be..db8050b81 100644 --- a/signac/synced_collections/buffers/file_buffered_collection.py +++ b/signac/synced_collections/buffers/file_buffered_collection.py @@ -364,9 +364,8 @@ def buffer_backend(cls, buffer_size=None, force_write=None, *args, **kwargs): """ if force_write is not None: warnings.warn( - DeprecationWarning( - "The force_write parameter is deprecated and will be removed in " - "signac 2.0. This functionality is no longer supported." - ) + "The force_write parameter is deprecated and will be removed in " + "signac 2.0. This functionality is no longer supported.", + FutureWarning, ) return cls._buffer_context(buffer_size) diff --git a/signac/warnings.py b/signac/warnings.py index 0132e4168..ba9d1b8e1 100644 --- a/signac/warnings.py +++ b/signac/warnings.py @@ -6,6 +6,8 @@ """Module for signac deprecation warnings.""" +# TODO: This can be removed in signac 2.0. We can use FutureWarning for this +# purpose going forward. class SignacDeprecationWarning(UserWarning): """Indicates the deprecation of a signac feature, API or behavior. From bb454b466b19e4e6c9538ad5ee988542ad5f9d02 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 20 Feb 2022 22:40:34 -0800 Subject: [PATCH 04/24] Deprecate or makes on potentially unnecessary schema APIs. --- signac/contrib/schema.py | 61 ++++++++++--------- tests/test_project.py | 9 ++- .../test_json_collection.py | 2 +- 3 files changed, 38 insertions(+), 34 deletions(-) diff --git a/signac/contrib/schema.py b/signac/contrib/schema.py index 2502bab29..eb900c08a 100644 --- a/signac/contrib/schema.py +++ b/signac/contrib/schema.py @@ -4,6 +4,7 @@ """Project Schema.""" import itertools +import warnings from collections import defaultdict as ddict from collections.abc import Mapping from numbers import Number @@ -42,6 +43,9 @@ def _collect_by_type(values): return values_by_type +# TODO: This function feels slightly out of place since it's never used here while it is used in +# import_export.py and project.py, but I see why it fits as "schema". May still make sense to move, +# though. def _build_job_statepoint_index(exclude_const, index): """Build index for job state points. @@ -103,7 +107,7 @@ def remove_dict_placeholder(x): yield statepoint_key, statepoint_values -class ProjectSchema: +class ProjectSchema(Mapping): """A description of a project's state point schema. Parameters @@ -118,6 +122,9 @@ def __init__(self, schema=None): schema = {} self._schema = schema + # TODO: If we move _collect_key_by_type to project.py we could just inline this logic when + # constructing the ProjectSchema and then this method wouldn't need to exist. It feels like an + # unnecessarily specialized method to be defined here. @classmethod def detect(cls, statepoint_index): """Detect Project's state point schema. @@ -267,43 +274,36 @@ def _repr_html_(self): output += "
" + str(self) + "
" return output + # TODO: This method can be removed in signac 2.0 once support for list keys + # is removed. def __contains__(self, key_or_keys): - if isinstance(key_or_keys, str): - return key_or_keys in self._schema - key_or_keys = ".".join(key_or_keys) + # NotOverride default __contains__ to support sequence and str inputs. + if not isinstance(key_or_keys, str): + warnings.warn( + "Support for checking nested keys in a schema using a list of keys is deprecated " + "and will be removed in signac 2.0. Construct the nested key using '.'.join(...) " + "instead.", + FutureWarning, + ) + key_or_keys = ".".join(key_or_keys) return key_or_keys in self._schema def __getitem__(self, key_or_keys): - if isinstance(key_or_keys, str): - return self._schema[key_or_keys] - return self._schema[".".join(key_or_keys)] + if not isinstance(key_or_keys, str): + warnings.warn( + "Support for checking nested keys in a schema using a list of keys is deprecated " + "and will be removed in signac 2.0. Construct the nested key using '.'.join(...) " + "instead.", + FutureWarning, + ) + key_or_keys = ".".join(key_or_keys) + return self._schema[key_or_keys] def __iter__(self): return iter(self._schema) - def keys(self): - """Return schema keys.""" - return self._schema.keys() - - def values(self): - """Return schema values.""" - return self._schema.values() - - def items(self): - """Return schema items.""" - return self._schema.items() - - def __eq__(self, other): - """Check if two schemas are the same. - - Returns - ------- - bool - True if both schemas have the same keys and values. - - """ - return self._schema == other._schema - + # TODO: We don't use this method anywhere. Is there a particular use case that is facilitated by + # keeping it, or should we deprecate? def difference(self, other, ignore_values=False): """Determine the difference between this and another project schema. @@ -332,6 +332,7 @@ def difference(self, other, ignore_values=False): ) return ret + # TODO: Same as difference, when do we need this? def __call__(self, jobs_or_statepoints): """Evaluate the schema for the given state points. diff --git a/tests/test_project.py b/tests/test_project.py index 259a296e8..50aade1d5 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -841,16 +841,19 @@ def test_schema(self): assert len(s) == 9 for k in "const", "const2.const3", "a", "b.b2", "c", "d", "e.e2", "f.f2": assert k in s - assert k.split(".") in s + with pytest.warns(FutureWarning): + assert k.split(".") in s # The following calls should not error out. s[k] - s[k.split(".")] + with pytest.warns(FutureWarning): + s[k.split(".")] repr(s) assert s.format() == str(s) s = self.project.detect_schema(exclude_const=True) assert len(s) == 7 assert "const" not in s - assert ("const2", "const3") not in s + with pytest.warns(FutureWarning): + assert ("const2", "const3") not in s assert "const2.const3" not in s assert type not in s["e"] diff --git a/tests/test_synced_collections/test_json_collection.py b/tests/test_synced_collections/test_json_collection.py index 4868afb7b..70cabe042 100644 --- a/tests/test_synced_collections/test_json_collection.py +++ b/tests/test_synced_collections/test_json_collection.py @@ -52,7 +52,7 @@ class TestJSONDict(JSONCollectionTest, SyncedDictTest): # See issue: https://github.com/glotzerlab/signac/issues/316. def test_keys_non_str_valid_type(self, synced_collection, testdata): for key in (0, None, True): - with pytest.deprecated_call(match="Use of.+as key is deprecated"): + with pytest.warns(FutureWarning, match="Use of.+as key is deprecated"): synced_collection[key] = testdata assert str(key) in synced_collection assert synced_collection[str(key)] == testdata From 5eea5e1ccff236f6d324e7e2dd77ed3f7094e13a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 20 Feb 2022 23:00:36 -0800 Subject: [PATCH 05/24] Deprecate unnecessary utility module. --- signac/core/utility.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/signac/core/utility.py b/signac/core/utility.py index a740e8b33..87293a21b 100644 --- a/signac/core/utility.py +++ b/signac/core/utility.py @@ -10,6 +10,10 @@ from ..version import __version__ +""" +THIS MODULE IS DEPRECATED! +""" + @deprecated( deprecated_in="1.3", @@ -41,6 +45,12 @@ def get_subject_from_certificate(fn_certificate): # noqa: D103, E261 return lines[0][len("subject=") :].strip() +@deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + details="Use the packaging package's Version object instead.", +) class Version(dict): """Utility class to manage revision control numbers.""" @@ -78,6 +88,12 @@ def __repr__(self): return "Version({})".format(",".join((f"{k}={v}" for k, v in self.items()))) +@deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + details="Use the packaging package's version.parse function instead.", +) def parse_version(version_str): """Parse a version number into a version object.""" p = re.compile( From 31d4267748542cbbcd3153aa6c89ec246d1371af Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 20 Feb 2022 23:04:49 -0800 Subject: [PATCH 06/24] Some notes. --- signac/cite.py | 5 +++++ signac/testing.py | 2 ++ 2 files changed, 7 insertions(+) diff --git a/signac/cite.py b/signac/cite.py index dbb8d3aa7..f3b4cadfb 100644 --- a/signac/cite.py +++ b/signac/cite.py @@ -8,6 +8,11 @@ from .version import __version__ +""" +THIS MODULE IS DEPRECATED! +""" + + ARXIV_BIBTEX = """@online{signac, author = {Carl S. Adorf and Paul M. Dodd and Sharon C. Glotzer}, title = {signac - A Simple Data Management Framework}, diff --git a/signac/testing.py b/signac/testing.py index 845ff1300..a899c5280 100644 --- a/signac/testing.py +++ b/signac/testing.py @@ -5,6 +5,8 @@ from itertools import cycle +# TODO: This feels like something that should be a test fixture or +# fixture-adjacent, not code that belongs in the main package. def init_jobs(project, nested=False, listed=False, heterogeneous=False): """Initialize a dataspace for testing purposes. From b26d83263f2ef3a5d98e102a2293163f68f64420 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 20 Feb 2022 23:05:19 -0800 Subject: [PATCH 07/24] Revert "Replace all DeprecationWarnings with FutureWarnings." This reverts commit 102ef00a4f7fe87a499df49bb9202bae5f9d573b. --- signac/contrib/project.py | 16 ++++++++-------- .../backends/collection_json.py | 4 ++-- .../buffers/file_buffered_collection.py | 7 ++++--- signac/warnings.py | 2 -- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/signac/contrib/project.py b/signac/contrib/project.py index 3a76242ec..b37e2184b 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -170,7 +170,7 @@ def find_job_ids(self, filter=None, doc_filter=None): if doc_filter: filter.update(doc_filter) elif doc_filter: - warnings.warn(DOC_FILTER_WARNING, FutureWarning) + warnings.warn(DOC_FILTER_WARNING, DeprecationWarning) filter = doc_filter return self._collection._find(filter) @@ -910,7 +910,7 @@ def detect_schema(self, exclude_const=False, subset=None, index=None): if index is None: index = self.index(include_job_document=False) else: - warnings.warn(INDEX_DEPRECATION_WARNING, FutureWarning) + warnings.warn(INDEX_DEPRECATION_WARNING, DeprecationWarning) if subset is not None: subset = {str(s) for s in subset} index = [doc for doc in index if doc["_id"] in subset] @@ -1022,7 +1022,7 @@ def _find_job_ids(self, filter=None, doc_filter=None, index=None): if index is None: filter = dict(parse_filter(_add_prefix("sp.", filter))) if doc_filter: - warnings.warn(DOC_FILTER_WARNING, FutureWarning) + warnings.warn(DOC_FILTER_WARNING, DeprecationWarning) filter.update(parse_filter(_add_prefix("doc.", doc_filter))) index = self.index(include_job_document=True) elif "doc" in _root_keys(filter): @@ -1030,7 +1030,7 @@ def _find_job_ids(self, filter=None, doc_filter=None, index=None): else: index = self._sp_index() else: - warnings.warn(INDEX_DEPRECATION_WARNING, FutureWarning) + warnings.warn(INDEX_DEPRECATION_WARNING, DeprecationWarning) return Collection(index, _trust=True)._find(filter) @@ -1070,7 +1070,7 @@ def find_jobs(self, filter=None, doc_filter=None): """ filter = dict(parse_filter(_add_prefix("sp.", filter))) if doc_filter: - warnings.warn(DOC_FILTER_WARNING, FutureWarning) + warnings.warn(DOC_FILTER_WARNING, DeprecationWarning) filter.update(parse_filter(_add_prefix("doc.", doc_filter))) return JobsCursor(self, filter) @@ -1512,7 +1512,7 @@ def create_linked_view(self, prefix=None, job_ids=None, index=None, path=None): """ if index is not None: - warnings.warn(INDEX_DEPRECATION_WARNING, FutureWarning) + warnings.warn(INDEX_DEPRECATION_WARNING, DeprecationWarning) from .linked_view import create_linked_view return create_linked_view(self, prefix, job_ids, index, path) @@ -1913,7 +1913,7 @@ def repair(self, fn_statepoints=None, index=None, job_ids=None): if index is not None: for doc in index: self._sp_cache[doc["signac_id"]] = doc["sp"] - warnings.warn(INDEX_DEPRECATION_WARNING, FutureWarning) + warnings.warn(INDEX_DEPRECATION_WARNING, DeprecationWarning) corrupted = [] for job_id in job_ids: try: @@ -2217,7 +2217,7 @@ def create_access_module(self, filename=None, main=True, master=None): """ if master is not None: warnings.warn( - "The parameter master has been renamed to main.", FutureWarning + "The parameter master has been renamed to main.", DeprecationWarning ) main = master diff --git a/signac/synced_collections/backends/collection_json.py b/signac/synced_collections/backends/collection_json.py index ca2b8fc06..ce4caa65b 100644 --- a/signac/synced_collections/backends/collection_json.py +++ b/signac/synced_collections/backends/collection_json.py @@ -49,7 +49,7 @@ def _str_key(key): warnings.warn( f"Use of {type(key).__name__} as key is deprecated " "and will be removed in version 2.0", - FutureWarning, + DeprecationWarning, ) key = str(key) return key @@ -146,7 +146,7 @@ def json_attr_dict_validator(data): warnings.warn( f"Use of {type(key).__name__} as key is deprecated " "and will be removed in version 2.0.", - FutureWarning, + DeprecationWarning, ) data[str(key)] = data.pop(key) else: diff --git a/signac/synced_collections/buffers/file_buffered_collection.py b/signac/synced_collections/buffers/file_buffered_collection.py index db8050b81..e7519f0be 100644 --- a/signac/synced_collections/buffers/file_buffered_collection.py +++ b/signac/synced_collections/buffers/file_buffered_collection.py @@ -364,8 +364,9 @@ def buffer_backend(cls, buffer_size=None, force_write=None, *args, **kwargs): """ if force_write is not None: warnings.warn( - "The force_write parameter is deprecated and will be removed in " - "signac 2.0. This functionality is no longer supported.", - FutureWarning, + DeprecationWarning( + "The force_write parameter is deprecated and will be removed in " + "signac 2.0. This functionality is no longer supported." + ) ) return cls._buffer_context(buffer_size) diff --git a/signac/warnings.py b/signac/warnings.py index ba9d1b8e1..0132e4168 100644 --- a/signac/warnings.py +++ b/signac/warnings.py @@ -6,8 +6,6 @@ """Module for signac deprecation warnings.""" -# TODO: This can be removed in signac 2.0. We can use FutureWarning for this -# purpose going forward. class SignacDeprecationWarning(UserWarning): """Indicates the deprecation of a signac feature, API or behavior. From 4f988650a7916ea1950e93e684c213186bbbc234 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 20 Feb 2022 23:11:38 -0800 Subject: [PATCH 08/24] Update comment. --- signac/contrib/schema.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/signac/contrib/schema.py b/signac/contrib/schema.py index eb900c08a..9b1e5c5b7 100644 --- a/signac/contrib/schema.py +++ b/signac/contrib/schema.py @@ -302,8 +302,6 @@ def __getitem__(self, key_or_keys): def __iter__(self): return iter(self._schema) - # TODO: We don't use this method anywhere. Is there a particular use case that is facilitated by - # keeping it, or should we deprecate? def difference(self, other, ignore_values=False): """Determine the difference between this and another project schema. @@ -332,7 +330,8 @@ def difference(self, other, ignore_values=False): ) return ret - # TODO: Same as difference, when do we need this? + # TODO: We don't use this method anywhere that I see. Is there a particular use case that is + # facilitated by keeping it, or should we deprecate? def __call__(self, jobs_or_statepoints): """Evaluate the schema for the given state points. From 5f61c44ce0ac653633baaa6e1d20138e9ccc899b Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Mon, 21 Feb 2022 11:58:28 -0800 Subject: [PATCH 09/24] Minor fixes. --- signac/contrib/job.py | 5 +++++ tests/test_synced_collections/test_json_collection.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index abda18ef9..bdda390b9 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -362,6 +362,11 @@ def _statepoint_filename(self): # TODO: Why is ws a property while workspace is a method? Why don't the # Project methods for workspace/root_directory also behave the same? We # should standardize this. + # TODO: The discrepancy between Project and Job in what is defined as a + # "workspace" is also slightly confusing. In view of moving towards a + # unified 3.0 API where both are subclasses of a common Directory or + # similar, we should aim to unify these APIs (although probably with an + # extremely long deprecation period like the entire lifetime of signac 2.0) @property def ws(self): """Alias for :meth:`~Job.workspace`.""" diff --git a/tests/test_synced_collections/test_json_collection.py b/tests/test_synced_collections/test_json_collection.py index 70cabe042..4868afb7b 100644 --- a/tests/test_synced_collections/test_json_collection.py +++ b/tests/test_synced_collections/test_json_collection.py @@ -52,7 +52,7 @@ class TestJSONDict(JSONCollectionTest, SyncedDictTest): # See issue: https://github.com/glotzerlab/signac/issues/316. def test_keys_non_str_valid_type(self, synced_collection, testdata): for key in (0, None, True): - with pytest.warns(FutureWarning, match="Use of.+as key is deprecated"): + with pytest.deprecated_call(match="Use of.+as key is deprecated"): synced_collection[key] = testdata assert str(key) in synced_collection assert synced_collection[str(key)] == testdata From 3d14baf45d52c7f47074e45c47b9fb06c1e42b1c Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 4 Mar 2022 16:54:38 -0800 Subject: [PATCH 10/24] Add new path properties and deprecate Project.root_directory, Job.ws, and Job.workspace. --- signac/contrib/job.py | 59 ++++++++++++++++++++++----------------- signac/contrib/project.py | 21 +++++++++----- 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index bdda390b9..32e4e0b5e 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -281,6 +281,9 @@ def __init__(self, project, statepoint=None, _id=None): def _initialize_lazy_properties(self): """Initialize all properties that are designed to be loaded lazily.""" + # TODO: Consider using functools.cached_property for these instead. That simplify our code, + # although at the expense of creating multiple locks (each cached_property has its own lock + # AFAICT from looking at the implementation). with self._lock: self._wd = None self._document = None @@ -335,22 +338,15 @@ def __repr__(self): self.__class__.__name__, repr(self._project), self.statepoint ) + @deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + details="Use Job.path instead.", + ) def workspace(self): - """Return the job's unique workspace directory. - - See :ref:`signac job -w ` for the command line equivalent. - - Returns - ------- - str - The path to the job's workspace directory. - - """ - if self._wd is None: - # We can rely on the project workspace to be well-formed, so just - # use str.join with os.sep instead of os.path.join for speed. - self._wd = os.sep.join((self._project.workspace(), self.id)) - return self._wd + """Alias for :meth:`~Job.path`.""" + return self.path @property def _statepoint_filename(self): @@ -359,18 +355,29 @@ def _statepoint_filename(self): # use str.join with os.sep instead of os.path.join for speed. return os.sep.join((self.workspace(), self.FN_MANIFEST)) - # TODO: Why is ws a property while workspace is a method? Why don't the - # Project methods for workspace/root_directory also behave the same? We - # should standardize this. - # TODO: The discrepancy between Project and Job in what is defined as a - # "workspace" is also slightly confusing. In view of moving towards a - # unified 3.0 API where both are subclasses of a common Directory or - # similar, we should aim to unify these APIs (although probably with an - # extremely long deprecation period like the entire lifetime of signac 2.0) - @property + # Decorated properties aren't supported: https://github.com/python/mypy/issues/1362 + @property # type: ignore + @deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + details="Use Job.path instead.", + ) def ws(self): - """Alias for :meth:`~Job.workspace`.""" - return self.workspace() + """Alias for :meth:`~Job.path`.""" + return self.path + + @property + def path(self): + """str: The path to the job directory. + + See :ref:`signac job -w ` for the command line equivalent. + """ + if self._wd is None: + # We can rely on the project workspace to be well-formed, so just + # use str.join with os.sep instead of os.path.join for speed. + self._wd = os.sep.join((self._project.workspace(), self.id)) + return self._wd def reset_statepoint(self, new_statepoint): """Overwrite the state point of this job while preserving job data. diff --git a/signac/contrib/project.py b/signac/contrib/project.py index b37e2184b..cbd3ae7c9 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -212,6 +212,9 @@ def __setitem__(self, key, value): def _invalidate_config_cache(project): """Invalidate cached properties derived from a project config.""" + # TODO: Consider using functools.cached_property for these instead. That simplify our code, + # although at the expense of creating multiple locks (each cached_property has its own lock + # AFAICT from looking at the implementation). project._id = None project._rd = None project._wd = None @@ -348,15 +351,19 @@ def config(self): """ return self._config + @deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + details="Use Project.path instead.", + ) def root_directory(self): - """Return the project's root directory. + """Alias for :meth:`~Project.path`.""" + return self.path - Returns - ------- - str - Path of project directory. - - """ + @property + def path(self): + """str: The path to the project directory.""" if self._rd is None: self._rd = self.config["project_dir"] return self._rd From 2ea0616c1a110cd272a191ef6689418be35fcff1 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 4 Mar 2022 16:58:03 -0800 Subject: [PATCH 11/24] Deprecate Job.reset_statepoint. --- signac/contrib/job.py | 27 +++++++++++++++++++-------- signac/contrib/project.py | 2 +- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index 32e4e0b5e..f11837e99 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -379,6 +379,12 @@ def path(self): self._wd = os.sep.join((self._project.workspace(), self.id)) return self._wd + @deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + details="Use job.statepoint = new_statepoint instead.", + ) def reset_statepoint(self, new_statepoint): """Overwrite the state point of this job while preserving job data. @@ -463,7 +469,13 @@ def update_statepoint(self, update, overwrite=False): @property def statepoint(self): - """Get the job's state point. + """Get or set the job's state point. + + Setting the state point to a different value will change the job id. + + For more information, see + `Modifying the State Point + `_. .. warning:: @@ -478,11 +490,16 @@ def statepoint(self): See :ref:`signac statepoint ` for the command line equivalent. + .. danger:: + + Use this function with caution! Resetting a job's state point + may sometimes be necessary, but can possibly lead to incoherent + data spaces. + Returns ------- dict Returns the job's state point. - """ with self._lock: if self._statepoint_requires_init: @@ -498,11 +515,6 @@ def statepoint(self): return self._statepoint - # TODO: If we really believe what `reset_statepoint`'s documentation says, - # I think we should deprecate this functionality. If resetting a statepoint - # is a dangerous operation, making it settable via a property is too easy, - # and moreover it's impossible to document effectively because property - # setters don't show up in docs. @statepoint.setter def statepoint(self, new_statepoint): """Assign a new state point to this job. @@ -511,7 +523,6 @@ def statepoint(self, new_statepoint): ---------- new_statepoint : dict The new state point to be assigned. - """ self.reset_statepoint(new_statepoint) diff --git a/signac/contrib/project.py b/signac/contrib/project.py index cbd3ae7c9..89daa79e0 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -1528,7 +1528,7 @@ def create_linked_view(self, prefix=None, job_ids=None, index=None, path=None): deprecated_in="1.3", removed_in="2.0", current_version=__version__, - details="Use job.reset_statepoint() instead.", + details="Use job.statepoint = new_statepoint instead.", ) def reset_statepoint(self, job, new_statepoint): """Overwrite the state point of this job while preserving job data. From 52b606fd346c1c733132a325bc34ef1748465d0a Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 4 Mar 2022 17:06:36 -0800 Subject: [PATCH 12/24] Add version guards for deprecated functionality. --- signac/contrib/schema.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/signac/contrib/schema.py b/signac/contrib/schema.py index 9b1e5c5b7..32ac72711 100644 --- a/signac/contrib/schema.py +++ b/signac/contrib/schema.py @@ -10,6 +10,10 @@ from numbers import Number from pprint import pformat +from packaging import version + +from ..version import __version__ + class _Vividict(dict): """A dict that returns an empty _Vividict for keys that are missing. @@ -278,6 +282,7 @@ def _repr_html_(self): # is removed. def __contains__(self, key_or_keys): # NotOverride default __contains__ to support sequence and str inputs. + assert version.parse(__version__) < version.parse("2.0.0") if not isinstance(key_or_keys, str): warnings.warn( "Support for checking nested keys in a schema using a list of keys is deprecated " @@ -289,6 +294,7 @@ def __contains__(self, key_or_keys): return key_or_keys in self._schema def __getitem__(self, key_or_keys): + assert version.parse(__version__) < version.parse("2.0.0") if not isinstance(key_or_keys, str): warnings.warn( "Support for checking nested keys in a schema using a list of keys is deprecated " From a741d0debddb55af05fe82eda7226226a8bd8cc3 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 4 Mar 2022 17:15:47 -0800 Subject: [PATCH 13/24] Deprecate ProjectSchema.detect. --- signac/contrib/project.py | 6 ++++-- signac/contrib/schema.py | 11 ++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/signac/contrib/project.py b/signac/contrib/project.py index 89daa79e0..03f4f5df9 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -41,7 +41,7 @@ from .hashing import calc_id from .indexing import MainCrawler, SignacProjectCrawler from .job import Job -from .schema import ProjectSchema +from .schema import ProjectSchema, _collect_by_type from .utility import _mkdir_p, _nested_dicts_to_dotted_keys, split_and_print_progress logger = logging.getLogger(__name__) @@ -924,7 +924,9 @@ def detect_schema(self, exclude_const=False, subset=None, index=None): statepoint_index = _build_job_statepoint_index( exclude_const=exclude_const, index=index ) - return ProjectSchema.detect(statepoint_index) + return ProjectSchema( + {key: _collect_by_type(value) for key, value in statepoint_index} + ) @deprecated( deprecated_in="1.3", diff --git a/signac/contrib/schema.py b/signac/contrib/schema.py index 32ac72711..f61a3bf6a 100644 --- a/signac/contrib/schema.py +++ b/signac/contrib/schema.py @@ -10,6 +10,7 @@ from numbers import Number from pprint import pformat +from deprecation import deprecated from packaging import version from ..version import __version__ @@ -41,6 +42,8 @@ def _collect_by_type(values): A mapping of types to a set of input values of that type. """ + # TODO: This function should be moved to project.py in version 2.0. + assert version.parse(__version__) < version.parse("2.0.0") values_by_type = ddict(set) for v in values: values_by_type[type(v)].add(v) @@ -126,9 +129,11 @@ def __init__(self, schema=None): schema = {} self._schema = schema - # TODO: If we move _collect_key_by_type to project.py we could just inline this logic when - # constructing the ProjectSchema and then this method wouldn't need to exist. It feels like an - # unnecessarily specialized method to be defined here. + @deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + ) @classmethod def detect(cls, statepoint_index): """Detect Project's state point schema. From 2266f7704195960eef870cf72f3a654c6aa00473 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 4 Mar 2022 17:23:35 -0800 Subject: [PATCH 14/24] Deprecate ProjectSchema.__call__. --- signac/contrib/schema.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/signac/contrib/schema.py b/signac/contrib/schema.py index f61a3bf6a..4971946b1 100644 --- a/signac/contrib/schema.py +++ b/signac/contrib/schema.py @@ -341,8 +341,11 @@ def difference(self, other, ignore_values=False): ) return ret - # TODO: We don't use this method anywhere that I see. Is there a particular use case that is - # facilitated by keeping it, or should we deprecate? + @deprecated( + deprecated_in="1.8", + removed_in="2.0", + current_version=__version__, + ) def __call__(self, jobs_or_statepoints): """Evaluate the schema for the given state points. From de2da7ab4b9dc217f0d11e03f44133f2a2fec50c Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 4 Mar 2022 17:30:52 -0800 Subject: [PATCH 15/24] Address various minor comments. --- signac/contrib/project.py | 2 +- signac/contrib/schema.py | 7 ++----- signac/core/utility.py | 4 ++-- tests/test_project.py | 10 ++++++---- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/signac/contrib/project.py b/signac/contrib/project.py index 03f4f5df9..fa15f1826 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -2629,7 +2629,7 @@ def __iter__(self): ) @deprecated( - deprecated_in="1.8", + deprecated_in="0.9.6", removed_in="2.0", current_version=__version__, details="Use next(iter(...)) instead.", diff --git a/signac/contrib/schema.py b/signac/contrib/schema.py index 4971946b1..7e35007c8 100644 --- a/signac/contrib/schema.py +++ b/signac/contrib/schema.py @@ -50,9 +50,6 @@ def _collect_by_type(values): return values_by_type -# TODO: This function feels slightly out of place since it's never used here while it is used in -# import_export.py and project.py, but I see why it fits as "schema". May still make sense to move, -# though. def _build_job_statepoint_index(exclude_const, index): """Build index for job state points. @@ -291,7 +288,7 @@ def __contains__(self, key_or_keys): if not isinstance(key_or_keys, str): warnings.warn( "Support for checking nested keys in a schema using a list of keys is deprecated " - "and will be removed in signac 2.0. Construct the nested key using '.'.join(...) " + "and will be removed in signac 2.0. Construct the nested key using '.'.join(keys) " "instead.", FutureWarning, ) @@ -303,7 +300,7 @@ def __getitem__(self, key_or_keys): if not isinstance(key_or_keys, str): warnings.warn( "Support for checking nested keys in a schema using a list of keys is deprecated " - "and will be removed in signac 2.0. Construct the nested key using '.'.join(...) " + "and will be removed in signac 2.0. Construct the nested key using '.'.join(keys) " "instead.", FutureWarning, ) diff --git a/signac/core/utility.py b/signac/core/utility.py index 87293a21b..083dc14c9 100644 --- a/signac/core/utility.py +++ b/signac/core/utility.py @@ -49,7 +49,7 @@ def get_subject_from_certificate(fn_certificate): # noqa: D103, E261 deprecated_in="1.8", removed_in="2.0", current_version=__version__, - details="Use the packaging package's Version object instead.", + details="Use packaging.version.Version instead.", ) class Version(dict): """Utility class to manage revision control numbers.""" @@ -92,7 +92,7 @@ def __repr__(self): deprecated_in="1.8", removed_in="2.0", current_version=__version__, - details="Use the packaging package's version.parse function instead.", + details="Use packaging.version.Version instead.", ) def parse_version(version_str): """Parse a version number into a version object.""" diff --git a/tests/test_project.py b/tests/test_project.py index 50aade1d5..f872ee5d9 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -841,12 +841,14 @@ def test_schema(self): assert len(s) == 9 for k in "const", "const2.const3", "a", "b.b2", "c", "d", "e.e2", "f.f2": assert k in s - with pytest.warns(FutureWarning): - assert k.split(".") in s + if "." in k: + with pytest.warns(FutureWarning): + assert k.split(".") in s # The following calls should not error out. s[k] - with pytest.warns(FutureWarning): - s[k.split(".")] + if "." in k: + with pytest.warns(FutureWarning): + s[k.split(".")] repr(s) assert s.format() == str(s) s = self.project.detect_schema(exclude_const=True) From 09dc77588783d1fc290cd7e3b0d661cd97ebaa95 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 5 Mar 2022 22:25:48 -0800 Subject: [PATCH 16/24] Address most PR comments. --- signac/contrib/job.py | 1 - signac/contrib/project.py | 39 ++++++++++++++++++--------------------- signac/core/utility.py | 2 +- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index f11837e99..c1ae92690 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -531,7 +531,6 @@ def sp(self): """Alias for :attr:`~Job.statepoint`.""" return self.statepoint - # TODO: Same concern as statepoint setter. @sp.setter def sp(self, new_sp): """Alias for :attr:`~Job.statepoint`.""" diff --git a/signac/contrib/project.py b/signac/contrib/project.py index 47768ed9b..da0e75faf 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -87,6 +87,19 @@ def get_indexes(root): _DEFAULT_PROJECT_NAME = None +class _CallableString(str): + # A string object that returns itself when called. Introduced temporarily + # to support deprecating Project.workspace, which will become a property. + def __call__(self): + assert version.parse(__version__) < version.parse("2.0") + warnings.warn( + "This method will soon become a property and should be used " + "without the call operator (parentheses).", + FutureWarning, + ) + return self + + class JobSearchIndex: """Search for specific jobs with filters. @@ -361,7 +374,7 @@ def config(self): details="Use Project.path instead.", ) def root_directory(self): - """Alias for :meth:`~Project.path`.""" + """Alias for :attr:`~Project.path`.""" return self.path @property @@ -371,34 +384,18 @@ def path(self): self._rd = self.config["project_dir"] return self._rd + @property def workspace(self): - """Return the project's workspace directory. - - The workspace defaults to `project_root/workspace`. - Configure this directory with the 'workspace_dir' - attribute. - If the specified directory is a relative path, - the absolute path is relative from the project's - root directory. - - .. note:: - The configuration will respect environment variables, - such as ``$HOME``. + """str: The project's workspace directory. See :ref:`signac project -w ` for the command line equivalent. - - Returns - ------- - str - Path of workspace directory. - """ if self._wd is None: wd = os.path.expandvars(self.config.get("workspace_dir", "workspace")) self._wd = ( wd if os.path.isabs(wd) else os.path.join(self.root_directory(), wd) ) - return self._wd + return _CallableString(self._wd) @deprecated( deprecated_in="1.3", @@ -2650,7 +2647,7 @@ def __iter__(self): def next(self): """Return the next element. - This function is deprecated. Users should use ``next(iter(..))`` instead. + This function is deprecated. Users should use ``next(iter(...))`` instead. .. deprecated:: 0.9.6 """ diff --git a/signac/core/utility.py b/signac/core/utility.py index 083dc14c9..353606c8e 100644 --- a/signac/core/utility.py +++ b/signac/core/utility.py @@ -49,7 +49,7 @@ def get_subject_from_certificate(fn_certificate): # noqa: D103, E261 deprecated_in="1.8", removed_in="2.0", current_version=__version__, - details="Use packaging.version.Version instead.", + details="Use packaging.version.parse instead.", ) class Version(dict): """Utility class to manage revision control numbers.""" From 57b73908a100da68b299cdfd23c6b331a8b527f0 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 5 Mar 2022 22:35:31 -0800 Subject: [PATCH 17/24] Add the --path parameter and deprecate --workspace. --- signac/__main__.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/signac/__main__.py b/signac/__main__.py index 75d9321f4..0ad3b5261 100644 --- a/signac/__main__.py +++ b/signac/__main__.py @@ -279,6 +279,12 @@ def main_job(args): if args.create: job.init() if args.workspace: + warnings.warn( + "The `-w/--workspace` parameter is deprecated. Use -p/--path instead", + FutureWarning, + ) + args.path = True + if args.path: print(job.workspace()) else: print(job) @@ -1244,6 +1250,12 @@ def main(): action="store_true", help="Print the job's workspace path instead of the job id.", ) + parser_job.add_argument( + "-p", + "--path", + action="store_true", + help="Print the job's path instead of the job id.", + ) parser_job.add_argument( "-c", "--create", From 2cf344a250d8d5e0d553bed0b852b74a8c005d3e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 13 Mar 2022 16:37:49 -0700 Subject: [PATCH 18/24] Address PR comments. --- signac/contrib/job.py | 15 ++++++++------- signac/contrib/project.py | 10 +++++----- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index c1ae92690..359ba56dc 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -285,7 +285,7 @@ def _initialize_lazy_properties(self): # although at the expense of creating multiple locks (each cached_property has its own lock # AFAICT from looking at the implementation). with self._lock: - self._wd = None + self._path = None self._document = None self._stores = None self._cwd = [] @@ -345,7 +345,7 @@ def __repr__(self): details="Use Job.path instead.", ) def workspace(self): - """Alias for :meth:`~Job.path`.""" + """Alias for :attr:`~Job.path`.""" return self.path @property @@ -355,7 +355,8 @@ def _statepoint_filename(self): # use str.join with os.sep instead of os.path.join for speed. return os.sep.join((self.workspace(), self.FN_MANIFEST)) - # Decorated properties aren't supported: https://github.com/python/mypy/issues/1362 + # Tell mypy to ignore type checking of the decorator because decorated + # properties aren't supported: https://github.com/python/mypy/issues/1362 @property # type: ignore @deprecated( deprecated_in="1.8", @@ -364,7 +365,7 @@ def _statepoint_filename(self): details="Use Job.path instead.", ) def ws(self): - """Alias for :meth:`~Job.path`.""" + """Alias for :attr:`~Job.path`.""" return self.path @property @@ -373,11 +374,11 @@ def path(self): See :ref:`signac job -w ` for the command line equivalent. """ - if self._wd is None: + if self._path is None: # We can rely on the project workspace to be well-formed, so just # use str.join with os.sep instead of os.path.join for speed. - self._wd = os.sep.join((self._project.workspace(), self.id)) - return self._wd + self._path = os.sep.join((self._project.workspace(), self.id)) + return self._path @deprecated( deprecated_in="1.8", diff --git a/signac/contrib/project.py b/signac/contrib/project.py index da0e75faf..360465a4a 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -380,9 +380,9 @@ def root_directory(self): @property def path(self): """str: The path to the project directory.""" - if self._rd is None: - self._rd = self.config["project_dir"] - return self._rd + if self._path is None: + self._path = self.config["project_dir"] + return self._path @property def workspace(self): @@ -787,8 +787,8 @@ def _contains_job_id(self, job_id): True if the job id is initialized for this project. """ - # We can rely on the project workspace to be well-formed, so just use - # os.sep.join instead of os.path.join for speed. + # We can rely on the project workspace to be well-formed, so use + # os.sep.join instead of os.path.join for performance. return os.path.exists(os.sep.join((self.workspace(), job_id))) def __contains__(self, job): From 76770a3c4ade1f931a38203963f311a7e319765e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sun, 13 Mar 2022 17:09:55 -0700 Subject: [PATCH 19/24] Correctly update internal variable. --- signac/contrib/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/signac/contrib/project.py b/signac/contrib/project.py index 360465a4a..ce8d53451 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -232,7 +232,7 @@ def _invalidate_config_cache(project): # although at the expense of creating multiple locks (each cached_property has its own lock # AFAICT from looking at the implementation). project._id = None - project._rd = None + project._path = None project._wd = None From adc4e65e83411279a6c7992a7f732b6606c4be7e Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 19 Mar 2022 14:13:34 -0700 Subject: [PATCH 20/24] Address PR comments. --- signac/__main__.py | 9 +++-- signac/contrib/import_export.py | 2 +- signac/contrib/indexing.py | 2 +- signac/contrib/job.py | 6 +-- signac/contrib/project.py | 44 +++++++++++----------- signac/sync.py | 2 +- tests/test_project.py | 67 ++++++++++++++++++--------------- 7 files changed, 68 insertions(+), 64 deletions(-) diff --git a/signac/__main__.py b/signac/__main__.py index 0ad3b5261..90ee8f7e7 100644 --- a/signac/__main__.py +++ b/signac/__main__.py @@ -258,7 +258,7 @@ def main_project(args): print(json.dumps(doc)) return if args.workspace: - print(project.workspace()) + print(project.workspace) else: print(project) @@ -280,7 +280,8 @@ def main_job(args): job.init() if args.workspace: warnings.warn( - "The `-w/--workspace` parameter is deprecated. Use -p/--path instead", + "The `-w/--workspace` parameter is deprecated as of version 1.8 and will be removed in " + "version 2.0. Use -p/--path instead", FutureWarning, ) args.path = True @@ -664,7 +665,7 @@ def _main_import_interactive(project, origin, args): project_id=project.get_id(), job_banner="", root_path=project.root_directory(), - workspace_path=project.workspace(), + workspace_path=project.workspace, size=len(project), origin=args.origin, ), @@ -1175,7 +1176,7 @@ def write_history_file(): project_id=project.id, job_banner=f"\nJob:\t\t{job.id}" if job is not None else "", root_path=project.root_directory(), - workspace_path=project.workspace(), + workspace_path=project.workspace, size=len(project), ), ) diff --git a/signac/contrib/import_export.py b/signac/contrib/import_export.py index fda1cdf85..12ef77a6b 100644 --- a/signac/contrib/import_export.py +++ b/signac/contrib/import_export.py @@ -758,7 +758,7 @@ def _crawl_directory_data_space(root, project, schema_function): """ # We compare paths to the 'realpath' of the project workspace to catch loops. - workspace_real_path = os.path.realpath(project.workspace()) + workspace_real_path = os.path.realpath(project.workspace) for path, dirs, _ in os.walk(root): sp = schema_function(path) diff --git a/signac/contrib/indexing.py b/signac/contrib/indexing.py index e2c70e812..4ca52243f 100644 --- a/signac/contrib/indexing.py +++ b/signac/contrib/indexing.py @@ -428,7 +428,7 @@ class SignacProjectCrawler(RegexFileCrawler): def __init__(self, root): from .project import get_project - root = get_project(root=root).workspace() + root = get_project(root=root).workspace self._statepoints = {} return super().__init__(root=root) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index 359ba56dc..dbaa1dedf 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -103,7 +103,7 @@ def _save(self): # Move the state point to an intermediate location as a backup. os.replace(self.filename, tmp_statepoint_file) try: - new_workspace = os.path.join(job._project.workspace(), new_id) + new_workspace = os.path.join(job._project.workspace, new_id) os.replace(job.workspace(), new_workspace) except OSError as error: os.replace(tmp_statepoint_file, self.filename) # rollback @@ -377,7 +377,7 @@ def path(self): if self._path is None: # We can rely on the project workspace to be well-formed, so just # use str.join with os.sep instead of os.path.join for speed. - self._path = os.sep.join((self._project.workspace(), self.id)) + self._path = os.sep.join((self._project.workspace, self.id)) return self._path @deprecated( @@ -817,7 +817,7 @@ def move(self, project): with self._lock: statepoint = self.statepoint() dst = project.open_job(statepoint) - _mkdir_p(project.workspace()) + _mkdir_p(project.workspace) try: os.replace(self.workspace(), dst.workspace()) except OSError as error: diff --git a/signac/contrib/project.py b/signac/contrib/project.py index ce8d53451..a97725eeb 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -91,7 +91,7 @@ class _CallableString(str): # A string object that returns itself when called. Introduced temporarily # to support deprecating Project.workspace, which will become a property. def __call__(self): - assert version.parse(__version__) < version.parse("2.0") + assert version.parse(__version__) < version.parse("2.0.0") warnings.warn( "This method will soon become a property and should be used " "without the call operator (parentheses).", @@ -300,9 +300,9 @@ def __init__(self, config=None): self._stores = None # Prepare Workspace Directory - if not os.path.isdir(self.workspace()): + if not os.path.isdir(self.workspace): try: - _mkdir_p(self.workspace()) + _mkdir_p(self.workspace) except OSError: logger.error( "Error occurred while trying to create " @@ -346,7 +346,7 @@ def _repr_html_(self): "

" + f"Project: {self.id}
" + f"Root: {self.root_directory()}
" - + f"Workspace: {self.workspace()}
" + + f"Workspace: {self.workspace}
" + f"Size: {len(self)}" + "

" + self.find_jobs()._repr_html_jobs() @@ -722,29 +722,27 @@ def _job_dirs(self): """ try: - for d in os.listdir(self.workspace()): + for d in os.listdir(self.workspace): if JOB_ID_REGEX.match(d): yield d except OSError as error: if error.errno == errno.ENOENT: - if os.path.islink(self.workspace()): + if os.path.islink(self.workspace): raise WorkspaceError( - f"The link '{self.workspace()}' pointing to the workspace is broken." + f"The link '{self.workspace}' pointing to the workspace is broken." ) - elif not os.path.isdir(os.path.dirname(self.workspace())): + elif not os.path.isdir(os.path.dirname(self.workspace)): logger.warning( "The path to the workspace directory " - "('{}') does not exist.".format( - os.path.dirname(self.workspace()) - ) + "('{}') does not exist.".format(os.path.dirname(self.workspace)) ) else: logger.info( - f"The workspace directory '{self.workspace()}' does not exist!" + f"The workspace directory '{self.workspace}' does not exist!" ) else: logger.error( - f"Unable to access the workspace directory '{self.workspace()}'." + f"Unable to access the workspace directory '{self.workspace}'." ) raise WorkspaceError(error) @@ -789,7 +787,7 @@ def _contains_job_id(self, job_id): """ # We can rely on the project workspace to be well-formed, so use # os.sep.join instead of os.path.join for performance. - return os.path.exists(os.sep.join((self.workspace(), job_id))) + return os.path.exists(os.sep.join((self.workspace, job_id))) def __contains__(self, job): """Determine whether a job is in the project's data space. @@ -1344,12 +1342,12 @@ def _get_statepoint_from_workspace(self, job_id): """ # We can rely on the project workspace to be well-formed, so just use # str.join with os.sep instead of os.path.join for speed. - fn_manifest = os.sep.join((self.workspace(), job_id, self.Job.FN_MANIFEST)) + fn_manifest = os.sep.join((self.workspace, job_id, self.Job.FN_MANIFEST)) try: with open(fn_manifest, "rb") as manifest: return json.loads(manifest.read().decode()) except (OSError, ValueError) as error: - if os.path.isdir(os.sep.join((self.workspace(), job_id))): + if os.path.isdir(os.sep.join((self.workspace, job_id))): logger.error( "Error while trying to access state " "point manifest file of job '{}': '{}'.".format(job_id, error) @@ -1935,8 +1933,8 @@ def repair(self, fn_statepoints=None, index=None, job_ids=None): "The job id of job '{}' is incorrect; " "it should be '{}'.".format(job_id, correct_id) ) - invalid_wd = os.path.join(self.workspace(), job_id) - correct_wd = os.path.join(self.workspace(), correct_id) + invalid_wd = os.path.join(self.workspace, job_id) + correct_wd = os.path.join(self.workspace, correct_id) try: os.replace(invalid_wd, correct_wd) except OSError as error: @@ -2002,7 +2000,7 @@ def _build_index(self, include_job_document=False): False). """ - wd = self.workspace() if self.Job is Job else None + wd = self.workspace if self.Job is Job else None for _id in self._find_job_ids(): doc = dict(_id=_id, sp=self._get_statepoint(_id)) if include_job_document: @@ -2157,7 +2155,7 @@ def index( """ if formats is None: - root = self.workspace() + root = self.workspace def _full_doc(doc): """Add `signac_id` and `root` to the index document. @@ -2276,8 +2274,8 @@ def temporary_project(self, name=None, dir=None): if name is None: name = os.path.join(self.id, str(uuid.uuid4())) if dir is None: - dir = self.workspace() - _mkdir_p(self.workspace()) # ensure workspace exists + dir = self.workspace + _mkdir_p(self.workspace) # ensure workspace exists with TemporaryProject(name=name, cls=type(self), dir=dir) as tmp_project: yield tmp_project @@ -2350,7 +2348,7 @@ def init_project(cls, name=None, root=None, workspace=None, make_dir=True): assert project.id == str(name) if workspace is not None: assert os.path.realpath(workspace) == os.path.realpath( - project.workspace() + project.workspace ) return project except AssertionError: diff --git a/signac/sync.py b/signac/sync.py index 880f42057..6191a8ae6 100644 --- a/signac/sync.py +++ b/signac/sync.py @@ -500,7 +500,7 @@ def sync_projects( # Setup data modification proxy proxy = _FileModifyProxy( - root=source.workspace(), + root=source.workspace, follow_symlinks=follow_symlinks, permissions=preserve_permissions, times=preserve_times, diff --git a/tests/test_project.py b/tests/test_project.py index cfdba3fdf..32d74b4d0 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -109,7 +109,12 @@ def test_root_directory(self): assert self._tmp_pr == self.project.root_directory() def test_workspace_directory(self): - assert self._tmp_wd == self.project.workspace() + assert self._tmp_wd == self.project.workspace + + def test_workspace_property(self): + with pytest.warns(FutureWarning): + ws = self.project.workspace() + assert ws == self.project.workspace def test_config_modification(self): # In-memory modification of the project configuration is @@ -121,10 +126,10 @@ def test_config_modification(self): def test_workspace_directory_with_env_variable(self): os.environ["SIGNAC_ENV_DIR_TEST"] = self._tmp_wd self.project.config["workspace_dir"] = "${SIGNAC_ENV_DIR_TEST}" - assert self._tmp_wd == self.project.workspace() + assert self._tmp_wd == self.project.workspace def test_workspace_directory_exists(self): - assert os.path.exists(self.project.workspace()) + assert os.path.exists(self.project.workspace) def test_fn(self): assert self.project.fn("test/abc") == os.path.join( @@ -233,21 +238,21 @@ def root_path(): # Returns 'C:\\' on Windows, '/' on other platforms return os.path.abspath(os.sep) - assert self.project.workspace() == norm_path(self._tmp_wd) + assert self.project.workspace == norm_path(self._tmp_wd) abs_path = os.path.join(root_path(), "path", "to", "workspace") self.project.config["workspace_dir"] = abs_path - assert self.project.workspace() == norm_path(abs_path) + assert self.project.workspace == norm_path(abs_path) rel_path = norm_path(os.path.join("path", "to", "workspace")) self.project.config["workspace_dir"] = rel_path - assert self.project.workspace() == norm_path( - os.path.join(self.project.root_directory(), self.project.workspace()) + assert self.project.workspace == norm_path( + os.path.join(self.project.root_directory(), self.project.workspace) ) def test_no_workspace_warn_on_find(self, caplog): - if os.path.exists(self.project.workspace()): - os.rmdir(self.project.workspace()) + if os.path.exists(self.project.workspace): + os.rmdir(self.project.workspace) with caplog.at_level(logging.INFO): list(self.project.find_jobs()) # Python < 3.8 will return 2 messages. @@ -258,7 +263,7 @@ def test_no_workspace_warn_on_find(self, caplog): @pytest.mark.skipif(WINDOWS, reason="Symbolic links are unsupported on Windows.") def test_workspace_broken_link_error_on_find(self): - wd = self.project.workspace() + wd = self.project.workspace os.symlink(wd + "~", self.project.fn("workspace-link")) self.project.config["workspace_dir"] = "workspace-link" with pytest.raises(WorkspaceError): @@ -267,13 +272,13 @@ def test_workspace_broken_link_error_on_find(self): def test_workspace_read_only_path(self): # Create file where workspace would be, thus preventing the creation # of the workspace directory. - if os.path.exists(self.project.workspace()): - os.rmdir(self.project.workspace()) - with open(os.path.join(self.project.workspace()), "w"): + if os.path.exists(self.project.workspace): + os.rmdir(self.project.workspace) + with open(os.path.join(self.project.workspace), "w"): pass with pytest.raises(OSError): # Ensure that the file is in place. - os.mkdir(self.project.workspace()) + os.mkdir(self.project.workspace) assert issubclass(WorkspaceError, OSError) @@ -285,7 +290,7 @@ def test_workspace_read_only_path(self): logging.disable(logging.NOTSET) assert not os.path.isdir(self._tmp_wd) - assert not os.path.isdir(self.project.workspace()) + assert not os.path.isdir(self.project.workspace) def test_find_job_ids(self): statepoints = [{"a": i} for i in range(5)] @@ -566,7 +571,7 @@ def test_rename_workspace(self): job.init() # First, we move the job to the wrong directory. wd = job.workspace() - wd_invalid = os.path.join(self.project.workspace(), "0" * 32) + wd_invalid = os.path.join(self.project.workspace, "0" * 32) os.replace(wd, wd_invalid) # Move to incorrect id. assert not os.path.exists(job.workspace()) @@ -1364,7 +1369,7 @@ def test_export_import_tarfile(self): with TarFile(name=target) as tarfile: for i in range(10): assert f"a/{i}" in tarfile.getnames() - os.replace(self.project.workspace(), self.project.workspace() + "~") + os.replace(self.project.workspace, self.project.workspace + "~") assert len(self.project) == 0 self.project.import_from(origin=target) assert len(self.project) == 10 @@ -1400,7 +1405,7 @@ def test_export_import_tarfile_zipped(self): for i in range(10): assert f"a/{i}" in tarfile.getnames() assert f"a/{i}/sub-dir/signac_statepoint.json" in tarfile.getnames() - os.replace(self.project.workspace(), self.project.workspace() + "~") + os.replace(self.project.workspace, self.project.workspace + "~") assert len(self.project) == 0 self.project.import_from(origin=target) assert len(self.project) == 10 @@ -1425,7 +1430,7 @@ def test_export_import_zipfile(self): for i in range(10): assert f"a/{i}/signac_statepoint.json" in zipfile.namelist() assert f"a/{i}/sub-dir/signac_statepoint.json" in zipfile.namelist() - os.replace(self.project.workspace(), self.project.workspace() + "~") + os.replace(self.project.workspace, self.project.workspace + "~") assert len(self.project) == 0 self.project.import_from(origin=target) assert len(self.project) == 10 @@ -1478,7 +1483,7 @@ def test_export_import_conflict_synced_with_args(self): assert len(self.project.import_from(prefix_data)) == 10 selection = list(self.project.find_jobs(dict(a=0))) - os.replace(self.project.workspace(), self.project.workspace() + "~") + os.replace(self.project.workspace, self.project.workspace + "~") assert len(self.project) == 0 assert ( len(self.project.import_from(prefix_data, sync=dict(selection=selection))) @@ -1637,10 +1642,10 @@ def test_import_own_project(self): for i in range(10): self.project.open_job(dict(a=i)).init() ids_before_export = {job.id for job in self.project.find_jobs()} - self.project.import_from(origin=self.project.workspace()) + self.project.import_from(origin=self.project.workspace) assert ids_before_export == {job.id for job in self.project.find_jobs()} with self.project.temporary_project() as tmp_project: - tmp_project.import_from(origin=self.project.workspace()) + tmp_project.import_from(origin=self.project.workspace) assert ids_before_export == {job.id for job in self.project.find_jobs()} assert len(tmp_project) == len(self.project) @@ -2290,19 +2295,19 @@ def test_get_project(self): signac.get_project(root=root) project = signac.init_project(name="testproject", root=root) assert project.id == "testproject" - assert project.workspace() == os.path.join(root, "workspace") + assert project.workspace == os.path.join(root, "workspace") assert project.root_directory() == root project = signac.Project.init_project(name="testproject", root=root) assert project.id == "testproject" - assert project.workspace() == os.path.join(root, "workspace") + assert project.workspace == os.path.join(root, "workspace") assert project.root_directory() == root project = signac.get_project(root=root) assert project.id == "testproject" - assert project.workspace() == os.path.join(root, "workspace") + assert project.workspace == os.path.join(root, "workspace") assert project.root_directory() == root project = signac.Project.get_project(root=root) assert project.id == "testproject" - assert project.workspace() == os.path.join(root, "workspace") + assert project.workspace == os.path.join(root, "workspace") assert project.root_directory() == root def test_get_project_all_printable_characters(self): @@ -2338,17 +2343,17 @@ def test_init(self): signac.get_project(root=root) project = signac.init_project(name="testproject", root=root) assert project.id == "testproject" - assert project.workspace() == os.path.join(root, "workspace") + assert project.workspace == os.path.join(root, "workspace") assert project.root_directory() == root # Second initialization should not make any difference. project = signac.init_project(name="testproject", root=root) project = signac.get_project(root=root) assert project.id == "testproject" - assert project.workspace() == os.path.join(root, "workspace") + assert project.workspace == os.path.join(root, "workspace") assert project.root_directory() == root project = signac.Project.get_project(root=root) assert project.id == "testproject" - assert project.workspace() == os.path.join(root, "workspace") + assert project.workspace == os.path.join(root, "workspace") assert project.root_directory() == root # Deviating initialization parameters should result in errors. with pytest.raises(RuntimeError): @@ -2410,7 +2415,7 @@ def test_get_job_invalid_workspace(self): # since no signac_statepoint.json exists. cwd = os.getcwd() try: - os.chdir(project.workspace()) + os.chdir(project.workspace) with pytest.raises(LookupError): project.get_job() with pytest.raises(LookupError): @@ -2473,7 +2478,7 @@ def test_get_job_symlink_other_project(self): job_a.init() job_b = project_b.open_job({"b": 1}) job_b.init() - symlink_path = os.path.join(project_b.workspace(), job_a._id) + symlink_path = os.path.join(project_b.workspace, job_a._id) os.symlink(job_a.ws, symlink_path) assert project_a.get_job(symlink_path) == job_a assert project_b.get_job(symlink_path) == job_a From 0d84123ebab7ff6dddfd87b03d9d9a4e924d618d Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 2 Apr 2022 15:29:07 -0700 Subject: [PATCH 21/24] PR comments. --- signac/__main__.py | 7 ++++- signac/contrib/import_export.py | 12 ++++----- signac/contrib/job.py | 28 ++++++++++---------- signac/contrib/linked_view.py | 4 +-- signac/contrib/project.py | 6 ++--- signac/sync.py | 14 +++++----- tests/test_job.py | 46 +++++++++++++++------------------ tests/test_project.py | 16 ++++++------ tests/test_shell.py | 4 +-- 9 files changed, 69 insertions(+), 68 deletions(-) diff --git a/signac/__main__.py b/signac/__main__.py index 90ee8f7e7..844a11637 100644 --- a/signac/__main__.py +++ b/signac/__main__.py @@ -248,6 +248,11 @@ def find_with_filter(args): def main_project(args): """Handle project subcommand.""" + warnings.warn( + "The `project` command is deprecated as of version 1.8 and will be removed in " + "version 2.0.", + FutureWarning, + ) project = get_project() if args.access: fn = project.create_access_module() @@ -286,7 +291,7 @@ def main_job(args): ) args.path = True if args.path: - print(job.workspace()) + print(job.path) else: print(job) diff --git a/signac/contrib/import_export.py b/signac/contrib/import_export.py index 12ef77a6b..a1e70e6f4 100644 --- a/signac/contrib/import_export.py +++ b/signac/contrib/import_export.py @@ -344,7 +344,7 @@ def _export_jobs(jobs, path, copytree): # path_function is checked for uniqueness inside _make_path_function # Determine export path for each job. - paths = {job.workspace(): path_function(job) for job in jobs} + paths = {job.path: path_function(job) for job in jobs} # Check leaf/node consistency _check_directory_structure_validity(paths.values()) @@ -765,7 +765,7 @@ def _crawl_directory_data_space(root, project, schema_function): if sp is not None: del dirs[:] # skip sub-directories job = project.open_job(sp) - dst = job.workspace() + dst = job.path if os.path.realpath(path) == os.path.realpath(dst): continue # skip (already part of the data space) elif os.path.realpath(path).startswith(workspace_real_path): @@ -792,7 +792,7 @@ def _copy_to_job_workspace(src, job, copytree): Destination filename. """ - dst = job.workspace() + dst = job.path try: copytree(src, dst) except OSError as error: @@ -913,7 +913,7 @@ def __call__(self, copytree=None): _mkdir_p(os.path.dirname(fn_dst)) with open(fn_dst, "wb") as dst: dst.write(self.zipfile.read(name)) - return self.job.workspace() + return self.job.path def __str__(self): return f"{type(self).__name__}({self.root} -> {self.job})" @@ -998,7 +998,7 @@ def read_sp_manifest_file(path): sp = schema_function(name) if sp is not None: job = project.open_job(sp) - if os.path.exists(job.workspace()): + if os.path.exists(job.path): raise DestinationExistsError(job) mappings[name] = job skip_subdirs.add(name) @@ -1145,7 +1145,7 @@ def read_sp_manifest_file(path): sp = schema_function(name) if sp is not None: job = project.open_job(sp) - if os.path.exists(job.workspace()): + if os.path.exists(job.path): raise DestinationExistsError(job) mappings[name] = job skip_subdirs.add(name) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index 773124539..f83b688d7 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -103,7 +103,7 @@ def _save(self): os.replace(self.filename, tmp_statepoint_file) try: new_workspace = os.path.join(job._project.workspace, new_id) - os.replace(job.workspace(), new_workspace) + os.replace(job.path, new_workspace) except OSError as error: os.replace(tmp_statepoint_file, self.filename) # rollback if error.errno in (errno.EEXIST, errno.ENOTEMPTY, errno.EACCES): @@ -325,8 +325,8 @@ def __eq__(self, other): if not isinstance(other, type(self)): return NotImplemented return self.id == other.id and os.path.realpath( - self.workspace() - ) == os.path.realpath(other.workspace()) + self.path + ) == os.path.realpath(other.path) def __str__(self): """Return the job's id.""" @@ -352,7 +352,7 @@ def _statepoint_filename(self): """Get the path of the state point file for this job.""" # We can rely on the job workspace to be well-formed, so just # use str.join with os.sep instead of os.path.join for speed. - return os.sep.join((self.workspace(), self.FN_MANIFEST)) + return os.sep.join((self.path, self.FN_MANIFEST)) # Tell mypy to ignore type checking of the decorator because decorated # properties aren't supported: https://github.com/python/mypy/issues/1362 @@ -561,7 +561,7 @@ def document(self): with self._lock: if self._document is None: self.init() - fn_doc = os.path.join(self.workspace(), self.FN_DOCUMENT) + fn_doc = os.path.join(self.path, self.FN_DOCUMENT) self._document = BufferedJSONAttrDict( filename=fn_doc, write_concern=True ) @@ -642,7 +642,7 @@ def stores(self): with self._lock: if self._stores is None: self.init() - self._stores = H5StoreManager(self.workspace()) + self._stores = H5StoreManager(self.path) return self.init()._stores @property @@ -718,7 +718,7 @@ def init(self, force=False): # Create the workspace directory if it does not exist. try: - _mkdir_p(self.workspace()) + _mkdir_p(self.path) except OSError: logger.error( "Error occurred while trying to create " @@ -751,10 +751,10 @@ def clear(self): """ try: - for fn in os.listdir(self.workspace()): + for fn in os.listdir(self.path): if fn in (self.FN_MANIFEST, self.FN_DOCUMENT): continue - path = os.path.join(self.workspace(), fn) + path = os.path.join(self.path, fn) if os.path.isfile(path): os.remove(path) elif os.path.isdir(path): @@ -785,7 +785,7 @@ def remove(self): """ with self._lock: try: - shutil.rmtree(self.workspace()) + shutil.rmtree(self.path) except OSError as error: if error.errno != errno.ENOENT: raise @@ -818,7 +818,7 @@ def move(self, project): dst = project.open_job(statepoint) _mkdir_p(project.workspace) try: - os.replace(self.workspace(), dst.workspace()) + os.replace(self.path, dst.path) except OSError as error: if error.errno == errno.ENOENT: raise RuntimeError( @@ -904,7 +904,7 @@ def fn(self, filename): The full workspace path of the file. """ - return os.path.join(self.workspace(), filename) + return os.path.join(self.path, filename) def isfile(self, filename): """Return True if file exists in the job's workspace. @@ -938,8 +938,8 @@ def open(self): """ self._cwd.append(os.getcwd()) self.init() - logger.info(f"Enter workspace '{self.workspace()}'.") - os.chdir(self.workspace()) + logger.info(f"Enter workspace '{self.path}'.") + os.chdir(self.path) def close(self): """Close the job and switch to the previous working directory.""" diff --git a/signac/contrib/linked_view.py b/signac/contrib/linked_view.py index a30bd5d95..aa000da53 100644 --- a/signac/contrib/linked_view.py +++ b/signac/contrib/linked_view.py @@ -103,10 +103,10 @@ def create_linked_view(project, prefix=None, job_ids=None, index=None, path=None links = {} for job in jobs: paths = os.path.join(path_function(job), "job") - links[paths] = job.workspace() + links[paths] = job.path if not links: # data space contains less than two elements for job in project.find_jobs(): - links["./job"] = job.workspace() + links["./job"] = job.path assert len(links) < 2 _check_directory_structure_validity(links.keys()) _update_view(prefix, links) diff --git a/signac/contrib/project.py b/signac/contrib/project.py index 0e898efd7..5cbe52a34 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -785,8 +785,8 @@ def _contains_job_id(self, job_id): True if the job id is initialized for this project. """ - # We can rely on the project workspace to be well-formed, so use - # os.sep.join instead of os.path.join for performance. + # We can rely on the project workspace to be well-formed, so just use + # str.join with os.sep instead of os.path.join for performance. return os.path.exists(os.sep.join((self.workspace, job_id))) def __contains__(self, job): @@ -1631,7 +1631,7 @@ def clone(self, job, copytree=shutil.copytree): """ dst = self.open_job(job.statepoint()) try: - copytree(job.workspace(), dst.workspace()) + copytree(job.path, dst.path) except OSError as error: if error.errno == errno.EEXIST: raise DestinationExistsError(dst) diff --git a/signac/sync.py b/signac/sync.py index 6191a8ae6..0c6e0d7dd 100644 --- a/signac/sync.py +++ b/signac/sync.py @@ -217,8 +217,8 @@ def _sync_job_workspaces( if exclude and any([re.match(p, fn) for p in exclude]): logger.debug(f"File named '{fn}' is skipped (excluded).") continue - fn_src = os.path.join(src.workspace(), subdir, fn) - fn_dst = os.path.join(dst.workspace(), subdir, fn) + fn_src = os.path.join(src.path, subdir, fn) + fn_dst = os.path.join(dst.path, subdir, fn) if os.path.isfile(fn_src): copy(fn_src, fn_dst) elif recursive: @@ -232,8 +232,8 @@ def _sync_job_workspaces( if strategy is None: raise FileSyncConflict(fn) else: - fn_src = os.path.join(src.workspace(), subdir, fn) - fn_dst = os.path.join(dst.workspace(), subdir, fn) + fn_src = os.path.join(src.path, subdir, fn) + fn_dst = os.path.join(dst.path, subdir, fn) if strategy(src, dst, os.path.join(subdir, fn)): copy(fn_src, fn_dst) else: @@ -335,7 +335,7 @@ def sync_jobs( """ # Check identity - if _identical_path(src.workspace(), dst.workspace()): + if _identical_path(src.path, dst.path): raise ValueError("Source and destination can't be the same!") # check src and dst compatiblity @@ -363,7 +363,7 @@ def sync_jobs( proxy = dry_run else: proxy = _FileModifyProxy( - root=src.workspace(), + root=src.path, follow_symlinks=follow_symlinks, permissions=preserve_permissions, times=preserve_times, @@ -376,7 +376,7 @@ def sync_jobs( else: logger.debug(f"Synchronizing job '{src}'...") - if os.path.isdir(src.workspace()): + if os.path.isdir(src.path): if not dry_run: dst.init() _sync_job_workspaces( diff --git a/tests/test_job.py b/tests/test_job.py index 2f223cbf3..5d579f668 100644 --- a/tests/test_job.py +++ b/tests/test_job.py @@ -132,14 +132,14 @@ class NonJob: def __init__(self, job): self.id = job.id - self._workspace = job.workspace() + self._workspace = job.path class JobSubclass(Job): """Minimal subclass that can be compared with Job objects.""" def __init__(self, job): self._id = job.id - self._workspace = job.workspace() + self._path = job.path def workspace(self): return self._workspace @@ -159,7 +159,7 @@ def workspace(self): def test_isfile(self): job = self.project.open_job({"a": 0}) fn = "test.txt" - fn_ = os.path.join(job.workspace(), fn) + fn_ = os.path.join(job.path, fn) assert not job.isfile(fn) job.init() assert not job.isfile(fn) @@ -526,21 +526,17 @@ def test_str(self): class TestJobOpenAndClosing(TestJobBase): def test_init(self): job = self.open_job(test_token) - assert not os.path.isdir(job.workspace()) + assert not os.path.isdir(job.path) job.init() - assert job.workspace() == job.ws - assert os.path.isdir(job.workspace()) - assert os.path.isdir(job.ws) - assert os.path.exists(os.path.join(job.workspace(), job.FN_MANIFEST)) + assert os.path.isdir(job.path) + assert os.path.exists(os.path.join(job.path, job.FN_MANIFEST)) def test_chained_init(self): job = self.open_job(test_token) - assert not os.path.isdir(job.workspace()) + assert not os.path.isdir(job.path) job = self.open_job(test_token).init() - assert job.workspace() == job.ws - assert os.path.isdir(job.workspace()) - assert os.path.isdir(job.ws) - assert os.path.exists(os.path.join(job.workspace(), job.FN_MANIFEST)) + assert os.path.isdir(job.path) + assert os.path.exists(os.path.join(job.path, job.FN_MANIFEST)) def test_construction(self): from signac import Project # noqa: F401 @@ -600,37 +596,37 @@ def test_open_job_recursive(self): cwd = rp(os.getcwd()) job = self.open_job(test_token) with job: - assert rp(job.workspace()) == rp(os.getcwd()) + assert rp(job.path) == rp(os.getcwd()) assert cwd == rp(os.getcwd()) with job: - assert rp(job.workspace()) == rp(os.getcwd()) + assert rp(job.path) == rp(os.getcwd()) os.chdir(self.project.root_directory()) assert cwd == rp(os.getcwd()) with job: - assert rp(job.workspace()) == rp(os.getcwd()) + assert rp(job.path) == rp(os.getcwd()) with job: - assert rp(job.workspace()) == rp(os.getcwd()) - assert rp(job.workspace()) == rp(os.getcwd()) + assert rp(job.path) == rp(os.getcwd()) + assert rp(job.path) == rp(os.getcwd()) assert cwd == rp(os.getcwd()) with job: - assert rp(job.workspace()) == rp(os.getcwd()) + assert rp(job.path) == rp(os.getcwd()) os.chdir(self.project.root_directory()) with job: - assert rp(job.workspace()) == rp(os.getcwd()) + assert rp(job.path) == rp(os.getcwd()) assert rp(os.getcwd()) == rp(self.project.root_directory()) assert cwd == rp(os.getcwd()) with job: job.close() assert cwd == rp(os.getcwd()) with job: - assert rp(job.workspace()) == rp(os.getcwd()) + assert rp(job.path) == rp(os.getcwd()) assert cwd == rp(os.getcwd()) assert cwd == rp(os.getcwd()) def test_corrupt_workspace(self): job = self.open_job(test_token) job.init() - fn_manifest = os.path.join(job.workspace(), job.FN_MANIFEST) + fn_manifest = os.path.join(job.path, job.FN_MANIFEST) with open(fn_manifest, "w") as file: file.write("corrupted") job2 = self.open_job(test_token) @@ -890,7 +886,7 @@ def test_remove(self): job.document[key] = d assert key in job.document assert len(job.document) == 1 - fn_test = os.path.join(job.workspace(), "test") + fn_test = os.path.join(job.path, "test") with open(fn_test, "w") as file: file.write("test") assert os.path.isfile(fn_test) @@ -1420,7 +1416,7 @@ def test_remove(self): job.data[key] = d assert key in job.data assert len(job.data) == 1 - fn_test = os.path.join(job.workspace(), "test") + fn_test = os.path.join(job.path, "test") with open(fn_test, "w") as file: file.write("test") assert os.path.isfile(fn_test) @@ -1906,7 +1902,7 @@ def test_remove(self): job.stores.test[key] = d assert key in job.stores.test assert len(job.stores.test) == 1 - fn_test = os.path.join(job.workspace(), "test") + fn_test = os.path.join(job.path, "test") with open(fn_test, "w") as file: file.write("test") assert os.path.isfile(fn_test) diff --git a/tests/test_project.py b/tests/test_project.py index d26e2b3cd..09c1c298b 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -570,10 +570,10 @@ def test_rename_workspace(self): job = self.project.open_job(dict(a=0)) job.init() # First, we move the job to the wrong directory. - wd = job.workspace() + wd = job.path wd_invalid = os.path.join(self.project.workspace, "0" * 32) os.replace(wd, wd_invalid) # Move to incorrect id. - assert not os.path.exists(job.workspace()) + assert not os.path.exists(job.path) try: logging.disable(logging.CRITICAL) @@ -1753,7 +1753,7 @@ def clean(filter=None): ) ) src = set( - map(lambda j: os.path.realpath(j.workspace()), self.project.find_jobs()) + map(lambda j: os.path.realpath(j.path), self.project.find_jobs()) ) assert len(all_links) == self.project.num_jobs() self.project.create_linked_view(prefix=view_prefix) @@ -1766,7 +1766,7 @@ def clean(filter=None): ) ) src = set( - map(lambda j: os.path.realpath(j.workspace()), self.project.find_jobs()) + map(lambda j: os.path.realpath(j.path), self.project.find_jobs()) ) assert src == dst # update with subset @@ -1788,7 +1788,7 @@ def clean(filter=None): all_links, ) ) - src = set(map(lambda j: os.path.realpath(j.workspace()), job_subset)) + src = set(map(lambda j: os.path.realpath(j.path), job_subset)) assert src == dst # some jobs removed clean({"b": 0}) @@ -1801,7 +1801,7 @@ def clean(filter=None): ) ) src = set( - map(lambda j: os.path.realpath(j.workspace()), self.project.find_jobs()) + map(lambda j: os.path.realpath(j.path), self.project.find_jobs()) ) assert src == dst # all jobs removed @@ -1815,7 +1815,7 @@ def clean(filter=None): ) ) src = set( - map(lambda j: os.path.realpath(j.workspace()), self.project.find_jobs()) + map(lambda j: os.path.realpath(j.path), self.project.find_jobs()) ) assert src == dst @@ -2479,7 +2479,7 @@ def test_get_job_symlink_other_project(self): job_b = project_b.open_job({"b": 1}) job_b.init() symlink_path = os.path.join(project_b.workspace, job_a._id) - os.symlink(job_a.ws, symlink_path) + os.symlink(job_a.path, symlink_path) assert project_a.get_job(symlink_path) == job_a assert project_b.get_job(symlink_path) == job_a assert signac.get_job(symlink_path) == job_a diff --git a/tests/test_shell.py b/tests/test_shell.py index 9751bc060..4bb4fc138 100644 --- a/tests/test_shell.py +++ b/tests/test_shell.py @@ -218,7 +218,7 @@ def test_view_single(self): for sp in sps: assert os.path.isdir("view/job") assert os.path.realpath("view/job") == os.path.realpath( - project.open_job(sp).workspace() + project.open_job(sp).path ) @pytest.mark.skipif(WINDOWS, reason="Symbolic links are unsupported on Windows.") @@ -235,7 +235,7 @@ def test_view(self): assert os.path.isdir("view/a/{}/job".format(sp["a"])) assert os.path.realpath( "view/a/{}/job".format(sp["a"]) - ) == os.path.realpath(project.open_job(sp).workspace()) + ) == os.path.realpath(project.open_job(sp).path) @pytest.mark.skipif(WINDOWS, reason="Symbolic links are unsupported on Windows.") def test_view_incomplete_path_spec(self): From 0effd92a1ab75ed55b067f6ca00e34947608ee20 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Sat, 2 Apr 2022 15:29:53 -0700 Subject: [PATCH 22/24] Fix deprecation import. --- signac/contrib/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/signac/contrib/schema.py b/signac/contrib/schema.py index 7e35007c8..cdf9273c7 100644 --- a/signac/contrib/schema.py +++ b/signac/contrib/schema.py @@ -10,9 +10,9 @@ from numbers import Number from pprint import pformat -from deprecation import deprecated from packaging import version +from ..common.deprecation import deprecated from ..version import __version__ From d146952f1696ea70ad0b9ce78d032597710b860c Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 2 Apr 2022 22:30:28 +0000 Subject: [PATCH 23/24] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- signac/contrib/job.py | 6 +++--- tests/test_project.py | 16 ++++------------ 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index f83b688d7..8ac34d641 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -324,9 +324,9 @@ def __hash__(self): def __eq__(self, other): if not isinstance(other, type(self)): return NotImplemented - return self.id == other.id and os.path.realpath( - self.path - ) == os.path.realpath(other.path) + return self.id == other.id and os.path.realpath(self.path) == os.path.realpath( + other.path + ) def __str__(self): """Return the job's id.""" diff --git a/tests/test_project.py b/tests/test_project.py index 09c1c298b..0787ba355 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -1752,9 +1752,7 @@ def clean(filter=None): all_links, ) ) - src = set( - map(lambda j: os.path.realpath(j.path), self.project.find_jobs()) - ) + src = set(map(lambda j: os.path.realpath(j.path), self.project.find_jobs())) assert len(all_links) == self.project.num_jobs() self.project.create_linked_view(prefix=view_prefix) all_links = list(_find_all_links(view_prefix)) @@ -1765,9 +1763,7 @@ def clean(filter=None): all_links, ) ) - src = set( - map(lambda j: os.path.realpath(j.path), self.project.find_jobs()) - ) + src = set(map(lambda j: os.path.realpath(j.path), self.project.find_jobs())) assert src == dst # update with subset job_subset = self.project.find_jobs({"b": 0}) @@ -1800,9 +1796,7 @@ def clean(filter=None): all_links, ) ) - src = set( - map(lambda j: os.path.realpath(j.path), self.project.find_jobs()) - ) + src = set(map(lambda j: os.path.realpath(j.path), self.project.find_jobs())) assert src == dst # all jobs removed clean() @@ -1814,9 +1808,7 @@ def clean(filter=None): all_links, ) ) - src = set( - map(lambda j: os.path.realpath(j.path), self.project.find_jobs()) - ) + src = set(map(lambda j: os.path.realpath(j.path), self.project.find_jobs())) assert src == dst @pytest.mark.skipif(WINDOWS, reason="Linked views unsupported on Windows.") From 4e9c77722a586a90d8dffa4003977562d56fe46c Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Mon, 11 Apr 2022 22:33:52 -0700 Subject: [PATCH 24/24] Remove cached_property TODOs. --- signac/contrib/job.py | 3 --- signac/contrib/project.py | 3 --- 2 files changed, 6 deletions(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index 8ac34d641..6d815a73c 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -280,9 +280,6 @@ def __init__(self, project, statepoint=None, _id=None): def _initialize_lazy_properties(self): """Initialize all properties that are designed to be loaded lazily.""" - # TODO: Consider using functools.cached_property for these instead. That simplify our code, - # although at the expense of creating multiple locks (each cached_property has its own lock - # AFAICT from looking at the implementation). with self._lock: self._path = None self._document = None diff --git a/signac/contrib/project.py b/signac/contrib/project.py index 5cbe52a34..7802a40b9 100644 --- a/signac/contrib/project.py +++ b/signac/contrib/project.py @@ -228,9 +228,6 @@ def __setitem__(self, key, value): def _invalidate_config_cache(project): """Invalidate cached properties derived from a project config.""" - # TODO: Consider using functools.cached_property for these instead. That simplify our code, - # although at the expense of creating multiple locks (each cached_property has its own lock - # AFAICT from looking at the implementation). project._id = None project._path = None project._wd = None