From 79d470559533142cbd080f23b82a01271e119808 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 19 Feb 2021 10:48:27 -0800 Subject: [PATCH 1/3] Switch protected keys from a sequence to a set for faster containment checks. --- signac/contrib/job.py | 6 +- .../backends/collection_json.py | 65 ++++++++++--------- .../data_types/attr_dict.py | 4 +- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index 703b30ca4..32b86d13c 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -9,7 +9,7 @@ import shutil from copy import deepcopy from json import JSONDecodeError -from typing import Tuple +from typing import FrozenSet from deprecation import deprecated @@ -46,7 +46,9 @@ class _StatePointDict(JSONAttrDict): job directory migrations. """ - _PROTECTED_KEYS: Tuple[str, ...] = JSONAttrDict._PROTECTED_KEYS + ("_jobs",) + _PROTECTED_KEYS: FrozenSet[str] = JSONAttrDict._PROTECTED_KEYS | frozenset( + ["_jobs"] + ) _all_validators = (json_attr_dict_validator,) def __init__( diff --git a/signac/synced_collections/backends/collection_json.py b/signac/synced_collections/backends/collection_json.py index 567e893a9..465692e0d 100644 --- a/signac/synced_collections/backends/collection_json.py +++ b/signac/synced_collections/backends/collection_json.py @@ -9,9 +9,8 @@ import uuid import warnings from collections.abc import Mapping, Sequence -from typing import Callable +from typing import Callable, FrozenSet from typing import Sequence as Sequence_t -from typing import Tuple from .. import SyncedCollection, SyncedDict, SyncedList from ..buffers.memory_buffered_collection import SharedMemoryFileBufferedCollection @@ -278,24 +277,26 @@ def _lock_id(self): # These are the common protected keys used by all JSONDict types. -_JSONDICT_PROTECTED_KEYS = ( - # These are all protected keys that are inherited from data type classes. - "_data", - "_name", - "_suspend_sync_", - "_load", - "_sync", - "_root", - "_validators", - "_all_validators", - "_load_and_save", - "_suspend_sync", - "_supports_threading", - "_LoadSaveType", - "registry", - # These keys are specific to the JSON backend. - "_filename", - "_write_concern", +_JSONDICT_PROTECTED_KEYS = frozenset( + [ + # These are all protected keys that are inherited from data type classes. + "_data", + "_name", + "_suspend_sync_", + "_load", + "_sync", + "_root", + "_validators", + "_all_validators", + "_load_and_save", + "_suspend_sync", + "_supports_threading", + "_LoadSaveType", + "registry", + # These keys are specific to the JSON backend. + "_filename", + "_write_concern", + ] ) @@ -342,7 +343,7 @@ class JSONDict(JSONCollection, SyncedDict): """ - _PROTECTED_KEYS: Tuple[str, ...] = _JSONDICT_PROTECTED_KEYS + _PROTECTED_KEYS: FrozenSet[str] = _JSONDICT_PROTECTED_KEYS def __init__( self, @@ -443,20 +444,22 @@ class BufferedJSONCollection(SerializedFileBufferedCollection, JSONCollection): # These are the keys common to buffer backends. -_BUFFERED_PROTECTED_KEYS = ( - "buffered", - "_is_buffered", - "_buffer_lock", - "_buffer_context", - "_buffered_collections", +_BUFFERED_PROTECTED_KEYS = frozenset( + [ + "buffered", + "_is_buffered", + "_buffer_lock", + "_buffer_context", + "_buffered_collections", + ] ) class BufferedJSONDict(BufferedJSONCollection, SyncedDict): """A buffered :class:`JSONDict`.""" - _PROTECTED_KEYS: Tuple[str, ...] = ( - _JSONDICT_PROTECTED_KEYS + _BUFFERED_PROTECTED_KEYS + _PROTECTED_KEYS: FrozenSet[str] = ( + _JSONDICT_PROTECTED_KEYS | _BUFFERED_PROTECTED_KEYS ) def __init__( @@ -521,8 +524,8 @@ class MemoryBufferedJSONCollection(SharedMemoryFileBufferedCollection, JSONColle class MemoryBufferedJSONDict(MemoryBufferedJSONCollection, SyncedDict): """A buffered :class:`JSONDict`.""" - _PROTECTED_KEYS: Tuple[str, ...] = ( - _JSONDICT_PROTECTED_KEYS + _BUFFERED_PROTECTED_KEYS + _PROTECTED_KEYS: FrozenSet[str] = ( + _JSONDICT_PROTECTED_KEYS | _BUFFERED_PROTECTED_KEYS ) def __init__( diff --git a/signac/synced_collections/data_types/attr_dict.py b/signac/synced_collections/data_types/attr_dict.py index 903550931..b7724e65f 100644 --- a/signac/synced_collections/data_types/attr_dict.py +++ b/signac/synced_collections/data_types/attr_dict.py @@ -9,7 +9,7 @@ simple mixin can be combined via inheritance without causing much difficulty. """ -from typing import Tuple +from typing import FrozenSet class AttrDict: @@ -34,7 +34,7 @@ class variable, which indicates known attributes of the object. This indication """ - _PROTECTED_KEYS: Tuple[str, ...] = () + _PROTECTED_KEYS: FrozenSet[str] = frozenset() def __getattr__(self, name): if name.startswith("__"): From fc16240fbc06368174953390d1cd5d1b220a09fb Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 19 Feb 2021 10:59:44 -0800 Subject: [PATCH 2/3] Change evaluation order of checks in setattr. --- signac/synced_collections/data_types/attr_dict.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/signac/synced_collections/data_types/attr_dict.py b/signac/synced_collections/data_types/attr_dict.py index b7724e65f..04f81c73f 100644 --- a/signac/synced_collections/data_types/attr_dict.py +++ b/signac/synced_collections/data_types/attr_dict.py @@ -49,13 +49,17 @@ def __setattr__(self, key, value): # the object has been fully instantiated. We may want to add a try # except in the else clause in case someone subclasses these and tries # to use d['foo'] inside a constructor prior to _data being defined. - if key.startswith("__") or key in self._PROTECTED_KEYS: + # The order of these checks assumes that setting protected keys will be + # much more common than setting dunder attributes. + if key in self._PROTECTED_KEYS or key.startswith("__"): super().__setattr__(key, value) else: self.__setitem__(key, value) def __delattr__(self, key): - if key.startswith("__") or key in self._PROTECTED_KEYS: + # The order of these checks assumes that setting protected keys will be + # much more common than setting dunder attributes. + if key in self._PROTECTED_KEYS or key.startswith("__"): super().__delattr__(key) else: self.__delitem__(key) From 2ef612dc953ce80e293017a68e5a3c0779b51139 Mon Sep 17 00:00:00 2001 From: Vyas Ramasubramani Date: Fri, 19 Feb 2021 13:00:39 -0800 Subject: [PATCH 3/3] Address PR comments. --- signac/contrib/job.py | 4 +--- signac/synced_collections/backends/collection_json.py | 8 ++++---- signac/synced_collections/data_types/attr_dict.py | 4 ++-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/signac/contrib/job.py b/signac/contrib/job.py index 32b86d13c..525b8f1c7 100644 --- a/signac/contrib/job.py +++ b/signac/contrib/job.py @@ -46,9 +46,7 @@ class _StatePointDict(JSONAttrDict): job directory migrations. """ - _PROTECTED_KEYS: FrozenSet[str] = JSONAttrDict._PROTECTED_KEYS | frozenset( - ["_jobs"] - ) + _PROTECTED_KEYS: FrozenSet[str] = JSONAttrDict._PROTECTED_KEYS.union(("_jobs",)) _all_validators = (json_attr_dict_validator,) def __init__( diff --git a/signac/synced_collections/backends/collection_json.py b/signac/synced_collections/backends/collection_json.py index 465692e0d..ce4caa65b 100644 --- a/signac/synced_collections/backends/collection_json.py +++ b/signac/synced_collections/backends/collection_json.py @@ -278,7 +278,7 @@ def _lock_id(self): # These are the common protected keys used by all JSONDict types. _JSONDICT_PROTECTED_KEYS = frozenset( - [ + ( # These are all protected keys that are inherited from data type classes. "_data", "_name", @@ -296,7 +296,7 @@ def _lock_id(self): # These keys are specific to the JSON backend. "_filename", "_write_concern", - ] + ) ) @@ -445,13 +445,13 @@ class BufferedJSONCollection(SerializedFileBufferedCollection, JSONCollection): # These are the keys common to buffer backends. _BUFFERED_PROTECTED_KEYS = frozenset( - [ + ( "buffered", "_is_buffered", "_buffer_lock", "_buffer_context", "_buffered_collections", - ] + ) ) diff --git a/signac/synced_collections/data_types/attr_dict.py b/signac/synced_collections/data_types/attr_dict.py index 04f81c73f..cc4be826b 100644 --- a/signac/synced_collections/data_types/attr_dict.py +++ b/signac/synced_collections/data_types/attr_dict.py @@ -57,8 +57,8 @@ def __setattr__(self, key, value): self.__setitem__(key, value) def __delattr__(self, key): - # The order of these checks assumes that setting protected keys will be - # much more common than setting dunder attributes. + # The order of these checks assumes that deleting protected keys will be + # much more common than deleting dunder attributes. if key in self._PROTECTED_KEYS or key.startswith("__"): super().__delattr__(key) else: