From 2fbb4521d4fb1257548f62341ac04b2dde58c0d0 Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 14:16:41 +0200 Subject: [PATCH 01/10] Result object now gives a detailed error message upon trying to call or hash an empty Result object --- TCutility/results/result.py | 48 +++++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/TCutility/results/result.py b/TCutility/results/result.py index 14bcbf0f..d4b5204d 100644 --- a/TCutility/results/result.py +++ b/TCutility/results/result.py @@ -4,6 +4,16 @@ class Result(dict): '''Class used for storing results from AMS calculations. The class is functionally a dictionary, but allows dot notation to access variables in the dictionary. The class works case-insensitively, but will retain the case of the key when it was first set.''' + def __repr__(self): + if not self: + return 'Result(empty)' + else: + return 'Result' + + def __call__(self): + head, method = '.'.join(self.get_parent_tree().split('.')[:-1]), self.get_parent_tree().split('.')[-1] + raise AttributeError(f'Tried to call method "{method}" from {head}, but {head} is empty') + def __getitem__(self, key): self.__set_empty(key) val = super().__getitem__(self.__get_case(key)) @@ -25,11 +35,23 @@ def __contains__(self, key): # Custom method to check if the key is defined in this object, case-insensitive. return key.lower() in [key_.lower() for key_ in self.keys()] + def __hash__(self): + raise KeyError(f'Tried to hash {self.get_parent_tree()}, but it is empty') + + def get_parent_tree(self): + if '__parent__' not in self: + return 'Head' + parent_names = self.__parent__.get_parent_tree() + parent_names += '.' + self.__name__ + return parent_names + def __set_empty(self, key): # This function checks if the key has been set. # If it has not, we create a new Result object and set it at the desired key if key not in self: val = Result() + val.__parent__ = self + val.__name__ = key self.__setitem__(key, val) def __get_case(self, key): @@ -40,12 +62,28 @@ def __get_case(self, key): return key_ return key + def prune(self): + '''Remove empty paths of this object. + ''' + items = list(self.items()) + for key, value in items: + try: + value.prune() + except AttributeError: + pass + + if not value: + del self[key] + if __name__ == '__main__': ret = Result() - ret.aDf.x = {'a': 1, 'b': 2} - print(ret.adf.x.a) - ret.ADF.y = 2345 - ret.dftb.z = 'hello' - print(ret) + ret.adf.x = {'a': 1, 'b': 2} + test_dict = {} + # test_dict[ret.adf.y] = 20 + # ret.adf.y.join() + # {ret.test: 123} + ret.__name = 'testname' + print(ret.__name) + print(ret.adf.y) From 18b7b46d8f4f3fbced71094e565b7c1ea14a72ae Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 14:40:39 +0200 Subject: [PATCH 02/10] Fixed an issue where __key__ formatted keys are part of the resulting dict. To fix this we have to make our own items method where we skip keys that are of the special format --- TCutility/results/result.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/TCutility/results/result.py b/TCutility/results/result.py index d4b5204d..e3497e42 100644 --- a/TCutility/results/result.py +++ b/TCutility/results/result.py @@ -4,16 +4,16 @@ class Result(dict): '''Class used for storing results from AMS calculations. The class is functionally a dictionary, but allows dot notation to access variables in the dictionary. The class works case-insensitively, but will retain the case of the key when it was first set.''' - def __repr__(self): - if not self: - return 'Result(empty)' - else: - return 'Result' def __call__(self): head, method = '.'.join(self.get_parent_tree().split('.')[:-1]), self.get_parent_tree().split('.')[-1] raise AttributeError(f'Tried to call method "{method}" from {head}, but {head} is empty') + def items(self): + original_keys = super().keys() + keys = [key for key in original_keys if not (key.startswith('__') and key.endswith('__'))] + return [(key, self[key]) for key in keys] + def __getitem__(self, key): self.__set_empty(key) val = super().__getitem__(self.__get_case(key)) @@ -38,6 +38,10 @@ def __contains__(self, key): def __hash__(self): raise KeyError(f'Tried to hash {self.get_parent_tree()}, but it is empty') + def __bool__(self): + '''Make sure that keys starting and ending in "__" are skipped''' + return len([key for key in self.keys() if not (key.startswith('__') and key.endswith('__'))]) > 0 + def get_parent_tree(self): if '__parent__' not in self: return 'Head' @@ -78,9 +82,12 @@ def prune(self): if __name__ == '__main__': ret = Result() + print(dict(ret.adf)) + print(bool(ret.adf)) + # ret.adf.x = {'a': 1, 'b': 2} + # ret.adf.system.atoms = [] + # ret.adf.system.atoms.append('test 1 2 3') - ret.adf.x = {'a': 1, 'b': 2} - test_dict = {} # test_dict[ret.adf.y] = 20 # ret.adf.y.join() # {ret.test: 123} From 5ac006188d742f770c72b4caee9e546f3cc42d65 Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 15:04:56 +0200 Subject: [PATCH 03/10] changed __set_empty to check if the key is not in keys instead of using __contains__. This is because __contains__ is now also checking if the key is empty --- TCutility/results/result.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/TCutility/results/result.py b/TCutility/results/result.py index e3497e42..8ab5a4ba 100644 --- a/TCutility/results/result.py +++ b/TCutility/results/result.py @@ -33,7 +33,7 @@ def __setattr__(self, key, val): def __contains__(self, key): # Custom method to check if the key is defined in this object, case-insensitive. - return key.lower() in [key_.lower() for key_ in self.keys()] + return key.lower() in [key_.lower() for key_ in self.keys()] and self[key] def __hash__(self): raise KeyError(f'Tried to hash {self.get_parent_tree()}, but it is empty') @@ -52,7 +52,7 @@ def get_parent_tree(self): def __set_empty(self, key): # This function checks if the key has been set. # If it has not, we create a new Result object and set it at the desired key - if key not in self: + if self.__get_case(key) not in self.keys(): val = Result() val.__parent__ = self val.__name__ = key From 13cabf75af655ee61d430c88a6c8b8b5b3fe7244 Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 15:05:17 +0200 Subject: [PATCH 04/10] Changed a mistake in a test-case, "history" variable should be empty --- test/test_results.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test_results.py b/test/test_results.py index 4f375ab3..a90550ef 100644 --- a/test/test_results.py +++ b/test/test_results.py @@ -35,7 +35,6 @@ def test_LOT4() -> None: assert res.level.summary == 'M06-2X/QZ4P' - def test_sections() -> None: res = results.read(j('test', 'fixtures', 'DFT_EDA')) - assert all(section in res for section in ['adf', 'engine', 'ams_version', 'history', 'is_multijob', 'molecule', 'status', 'timing']) + assert all(section in res for section in ['adf', 'engine', 'ams_version', 'is_multijob', 'molecule', 'status', 'timing']) From 0b40811d321b9a1b627b3c213c0a8a836d30f2b2 Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 15:05:29 +0200 Subject: [PATCH 05/10] Added test-cases for the Result object --- test/test_result_class.py | 50 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/test_result_class.py diff --git a/test/test_result_class.py b/test/test_result_class.py new file mode 100644 index 00000000..7e990cc3 --- /dev/null +++ b/test/test_result_class.py @@ -0,0 +1,50 @@ +from TCutility import results + +def test_init(): + res = results.result.Result() + + +def test_assign_simple(): + res = results.result.Result() + res.a = 10 + assert res.a == 10 + + +def test_contains(): + res = results.result.Result() + res.a = 10 + assert 'a' in res + + +def test_contains2(): + res = results.result.Result() + res.a.b = 10 + assert 'a' in res + + +def test_contains3(): + res = results.result.Result() + res.a.b = 10 + assert 'b' in res.a + + +def test_contains_neg(): + res = results.result.Result() + assert 'a' not in res + + +def test_contains_neg2(): + res = results.result.Result() + assert 'b' not in res.a + + +def test_contains_neg3(): + res = results.result.Result() + res.a.b = 10 + assert 'b' not in res + + +def test_contains_neg3(): + res = results.result.Result() + res.a + assert 'a' not in res From aa0108b60086ce15d7dd4e6fe6a6c69fc9c21b33 Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 15:11:23 +0200 Subject: [PATCH 06/10] Added some docstrings --- TCutility/results/result.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/TCutility/results/result.py b/TCutility/results/result.py index 8ab5a4ba..eefeda92 100644 --- a/TCutility/results/result.py +++ b/TCutility/results/result.py @@ -6,10 +6,14 @@ class Result(dict): The class works case-insensitively, but will retain the case of the key when it was first set.''' def __call__(self): + '''Calling of a dictionary subclass should not be possible, instead we raise an error with information about the key and method that were attempted to be called.''' head, method = '.'.join(self.get_parent_tree().split('.')[:-1]), self.get_parent_tree().split('.')[-1] raise AttributeError(f'Tried to call method "{method}" from {head}, but {head} is empty') def items(self): + '''We override the items method from dict in order to skip certain keys. We want to hide keys starting and ending + with dunders, as they should not be exposed to the user. + ''' original_keys = super().keys() keys = [key for key in original_keys if not (key.startswith('__') and key.endswith('__'))] return [(key, self[key]) for key in keys] @@ -32,10 +36,13 @@ def __setattr__(self, key, val): self.__setitem__(key, val) def __contains__(self, key): - # Custom method to check if the key is defined in this object, case-insensitive. + # Custom method to check if the key is defined in this object and is also non-empty, case-insensitive. return key.lower() in [key_.lower() for key_ in self.keys()] and self[key] def __hash__(self): + '''Hashing of a dictionary subclass should not be possible, instead we should raise an error to let the user know + that they made a mistake. Also give information of which key was being read. + ''' raise KeyError(f'Tried to hash {self.get_parent_tree()}, but it is empty') def __bool__(self): @@ -43,8 +50,11 @@ def __bool__(self): return len([key for key in self.keys() if not (key.startswith('__') and key.endswith('__'))]) > 0 def get_parent_tree(self): + '''Method to get the path from this object to the parent object. The result is presented in a formatted string''' + # every parent except the top-most parent has defined a __parent__ attribute if '__parent__' not in self: return 'Head' + # iteratively build the tree using the __name__ attribute. parent_names = self.__parent__.get_parent_tree() parent_names += '.' + self.__name__ return parent_names @@ -54,6 +64,7 @@ def __set_empty(self, key): # If it has not, we create a new Result object and set it at the desired key if self.__get_case(key) not in self.keys(): val = Result() + # we also keep track of the parent of this object and also the name it was assigned to for later bookkeeping val.__parent__ = self val.__name__ = key self.__setitem__(key, val) From 804a1c07c04729215305c6c85de1038561cc3676 Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 15:11:57 +0200 Subject: [PATCH 07/10] Renamed one test-case --- test/test_result_class.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_result_class.py b/test/test_result_class.py index 7e990cc3..7f9b305b 100644 --- a/test/test_result_class.py +++ b/test/test_result_class.py @@ -44,7 +44,7 @@ def test_contains_neg3(): assert 'b' not in res -def test_contains_neg3(): +def test_contains_neg4(): res = results.result.Result() res.a assert 'a' not in res From cf04f0cd474280465d498da54d1624eb9028c46e Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 15:14:01 +0200 Subject: [PATCH 08/10] Added a noqa to appease Ruff --- test/test_result_class.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_result_class.py b/test/test_result_class.py index 7f9b305b..20d915c5 100644 --- a/test/test_result_class.py +++ b/test/test_result_class.py @@ -1,7 +1,7 @@ from TCutility import results def test_init(): - res = results.result.Result() + res = results.result.Result() # noqa F841 def test_assign_simple(): From cdec1c58e03369acf3bd3a933a30fd44ec0a1047 Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Mon, 16 Oct 2023 16:27:20 +0200 Subject: [PATCH 09/10] Added more test-cases --- test/test_result_class.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/test/test_result_class.py b/test/test_result_class.py index 20d915c5..4afb6808 100644 --- a/test/test_result_class.py +++ b/test/test_result_class.py @@ -4,12 +4,32 @@ def test_init(): res = results.result.Result() # noqa F841 -def test_assign_simple(): +def test_assign(): res = results.result.Result() res.a = 10 assert res.a == 10 +def test_assign2(): + res = results.result.Result() + res.a.b = 10 + assert res.a.b == 10 + + +def test_assign3(): + res = results.result.Result() + res.a.b = 10 + res.a.c = 20 + assert res.a.b == 10 and res.a.c == 20 + + +def test_assign4(): + res = results.result.Result() + res.a.lst = [] + res.a.lst.append(10) + assert len(res.a.lst) == 1 + + def test_contains(): res = results.result.Result() res.a = 10 From 63373769a2699f0034aecd7d0a5ae2e6cabf9cef Mon Sep 17 00:00:00 2001 From: Yuman Hordijk Date: Tue, 17 Oct 2023 13:46:34 +0200 Subject: [PATCH 10/10] Overriding keys and __str__. This is because "hidden" keys are still shown in the original dict __str__ function. To print hidden keys, please use the __repr__ method. --- TCutility/results/result.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/TCutility/results/result.py b/TCutility/results/result.py index eefeda92..83b81167 100644 --- a/TCutility/results/result.py +++ b/TCutility/results/result.py @@ -10,15 +10,23 @@ def __call__(self): head, method = '.'.join(self.get_parent_tree().split('.')[:-1]), self.get_parent_tree().split('.')[-1] raise AttributeError(f'Tried to call method "{method}" from {head}, but {head} is empty') + def __str__(self): + '''Override str method to prevent printing of hidden keys. You can still print them if you call repr instead of str.''' + return '{' + ', '.join([f'{key}: {str(val)}' for key, val in self.items()]) + '}' + def items(self): '''We override the items method from dict in order to skip certain keys. We want to hide keys starting and ending with dunders, as they should not be exposed to the user. ''' + return [(key, self[key]) for key in self.keys()] + + def keys(self): original_keys = super().keys() - keys = [key for key in original_keys if not (key.startswith('__') and key.endswith('__'))] - return [(key, self[key]) for key in keys] + return [key for key in original_keys if not (key.startswith('__') and key.endswith('__'))] def __getitem__(self, key): + if key.startswith('__') and key.endswith('__'): + return None self.__set_empty(key) val = super().__getitem__(self.__get_case(key)) return val @@ -93,15 +101,16 @@ def prune(self): if __name__ == '__main__': ret = Result() - print(dict(ret.adf)) - print(bool(ret.adf)) - # ret.adf.x = {'a': 1, 'b': 2} + # print(ret.adf) + # print(dict(ret.adf)) + # print(bool(ret.adf)) + ret.adf.x = {'a': 1, 'b': 2} # ret.adf.system.atoms = [] # ret.adf.system.atoms.append('test 1 2 3') # test_dict[ret.adf.y] = 20 # ret.adf.y.join() # {ret.test: 123} - ret.__name = 'testname' - print(ret.__name) - print(ret.adf.y) + # ret.__name__ = 'testname' + # print(ret.__name__) + print(repr(ret))