Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional deprecations #685

Merged
merged 29 commits into from
Apr 12, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
dace571
Remove some unnecessary code in project.py.
vyasr Feb 21, 2022
a530fef
Make notes on possible deprecations/changes in job.py.
vyasr Feb 21, 2022
102ef00
Replace all DeprecationWarnings with FutureWarnings.
vyasr Feb 21, 2022
bb454b4
Deprecate or makes on potentially unnecessary schema APIs.
vyasr Feb 21, 2022
5eea5e1
Deprecate unnecessary utility module.
vyasr Feb 21, 2022
31d4267
Some notes.
vyasr Feb 21, 2022
b26d832
Revert "Replace all DeprecationWarnings with FutureWarnings."
vyasr Feb 21, 2022
4f98865
Update comment.
vyasr Feb 21, 2022
5f61c44
Minor fixes.
vyasr Feb 21, 2022
3d14baf
Add new path properties and deprecate Project.root_directory, Job.ws,…
vyasr Mar 5, 2022
2ea0616
Deprecate Job.reset_statepoint.
vyasr Mar 5, 2022
52b606f
Add version guards for deprecated functionality.
vyasr Mar 5, 2022
a741d0d
Deprecate ProjectSchema.detect.
vyasr Mar 5, 2022
2266f77
Deprecate ProjectSchema.__call__.
vyasr Mar 5, 2022
de2da7a
Address various minor comments.
vyasr Mar 5, 2022
d4a347a
Merge remote-tracking branch 'origin/master' into additional_deprecat…
vyasr Mar 5, 2022
09dc775
Address most PR comments.
vyasr Mar 6, 2022
57b7390
Add the --path parameter and deprecate --workspace.
vyasr Mar 6, 2022
2cf344a
Address PR comments.
vyasr Mar 13, 2022
e89c1be
Merge remote-tracking branch 'origin/master' into additional_deprecat…
vyasr Mar 13, 2022
76770a3
Correctly update internal variable.
vyasr Mar 14, 2022
adc4e65
Address PR comments.
vyasr Mar 19, 2022
85bf881
Merge branch 'master' into additional_deprecations
bdice Mar 28, 2022
8e28bb1
Merge remote-tracking branch 'origin/master' into additional_deprecat…
vyasr Apr 2, 2022
0d84123
PR comments.
vyasr Apr 2, 2022
0effd92
Fix deprecation import.
vyasr Apr 2, 2022
d146952
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 2, 2022
692f181
Merge remote-tracking branch 'origin/master' into additional_deprecat…
vyasr Apr 12, 2022
4e9c777
Remove cached_property TODOs.
vyasr Apr 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions signac/cite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
14 changes: 14 additions & 0 deletions signac/contrib/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,14 @@ 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)
vyasr marked this conversation as resolved.
Show resolved Hide resolved
@property
def ws(self):
"""Alias for :meth:`~Job.workspace`."""
Expand Down Expand Up @@ -483,6 +491,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.
vyasr marked this conversation as resolved.
Show resolved Hide resolved
@statepoint.setter
def statepoint(self, new_statepoint):
"""Assign a new state point to this job.
Expand All @@ -500,6 +513,7 @@ def sp(self):
"""Alias for :attr:`~Job.statepoint`."""
return self.statepoint

# TODO: Same concern as statepoint setter.
vyasr marked this conversation as resolved.
Show resolved Hide resolved
@sp.setter
def sp(self, new_sp):
"""Alias for :attr:`~Job.statepoint`."""
Expand Down
42 changes: 19 additions & 23 deletions signac/contrib/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
vyasr marked this conversation as resolved.
Show resolved Hide resolved

# Ensure that the project id is configured.
if self.id is None:
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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.

Expand All @@ -763,15 +756,16 @@ def num_jobs(self):
Count of initialized jobs.

"""
return len(self)

def __len__(self):
vyasr marked this conversation as resolved.
Show resolved Hide resolved
# We simply count the the number of valid directories and avoid building a list
# for improved performance.
i = 0
for i, _ in enumerate(self._job_dirs(), 1):
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.

Expand All @@ -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.
vyasr marked this conversation as resolved.
Show resolved Hide resolved
return os.path.exists(os.sep.join((self.workspace(), job_id)))
vyasr marked this conversation as resolved.
Show resolved Hide resolved
vyasr marked this conversation as resolved.
Show resolved Hide resolved

def __contains__(self, job):
Expand Down Expand Up @@ -2625,17 +2619,19 @@ def __iter__(self):
self._project, self._project._find_job_ids(self._filter)
)

@deprecated(
deprecated_in="1.8",
bdice marked this conversation as resolved.
Show resolved Hide resolved
removed_in="2.0",
current_version=__version__,
details="Use next(iter(...)) instead.",
)
def next(self):
"""Return the next element.

This function is deprecated. Users should use ``next(iter(..))`` instead.
vyasr marked this conversation as resolved.
Show resolved Hide resolved
.. 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:
Expand Down
60 changes: 30 additions & 30 deletions signac/contrib/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
vyasr marked this conversation as resolved.
Show resolved Hide resolved
def _build_job_statepoint_index(exclude_const, index):
"""Build index for job state points.

Expand Down Expand Up @@ -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
Expand All @@ -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.
vyasr marked this conversation as resolved.
Show resolved Hide resolved
@classmethod
def detect(cls, statepoint_index):
"""Detect Project's state point schema.
Expand Down Expand Up @@ -267,43 +274,34 @@ def _repr_html_(self):
output += "<pre>" + str(self) + "</pre>"
return output

# TODO: This method can be removed in signac 2.0 once support for list keys
vyasr marked this conversation as resolved.
Show resolved Hide resolved
# 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):
vyasr marked this conversation as resolved.
Show resolved Hide resolved
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(...) "
vyasr marked this conversation as resolved.
Show resolved Hide resolved
"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):
vyasr marked this conversation as resolved.
Show resolved Hide resolved
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(...) "
vyasr marked this conversation as resolved.
Show resolved Hide resolved
"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

def difference(self, other, ignore_values=False):
"""Determine the difference between this and another project schema.

Expand Down Expand Up @@ -332,6 +330,8 @@ 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?
vyasr marked this conversation as resolved.
Show resolved Hide resolved
def __call__(self, jobs_or_statepoints):
"""Evaluate the schema for the given state points.

Expand Down
16 changes: 16 additions & 0 deletions signac/core/utility.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

from ..version import __version__

"""
THIS MODULE IS DEPRECATED!
"""


@deprecated(
deprecated_in="1.3",
Expand Down Expand Up @@ -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.",
vyasr marked this conversation as resolved.
Show resolved Hide resolved
)
class Version(dict):
"""Utility class to manage revision control numbers."""

Expand Down Expand Up @@ -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.",
vyasr marked this conversation as resolved.
Show resolved Hide resolved
)
def parse_version(version_str):
"""Parse a version number into a version object."""
p = re.compile(
Expand Down
2 changes: 2 additions & 0 deletions signac/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
vyasr marked this conversation as resolved.
Show resolved Hide resolved
def init_jobs(project, nested=False, listed=False, heterogeneous=False):
"""Initialize a dataspace for testing purposes.

Expand Down
9 changes: 6 additions & 3 deletions tests/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
vyasr marked this conversation as resolved.
Show resolved Hide resolved
# The following calls should not error out.
s[k]
s[k.split(".")]
with pytest.warns(FutureWarning):
s[k.split(".")]
vyasr marked this conversation as resolved.
Show resolved Hide resolved
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"]

Expand Down