From 6e92120d7fedc9603d6a8cab41d836fbfb8d3f6f Mon Sep 17 00:00:00 2001 From: Pat Gunn Date: Tue, 5 Dec 2023 12:01:42 -0500 Subject: [PATCH] CNMFParams: more typing, some docs/message tweaks, mark set_if_not_exists in set method as deprecated --- caiman/source_extraction/cnmf/params.py | 26 +++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/caiman/source_extraction/cnmf/params.py b/caiman/source_extraction/cnmf/params.py index 14779894f..90d788093 100644 --- a/caiman/source_extraction/cnmf/params.py +++ b/caiman/source_extraction/cnmf/params.py @@ -984,16 +984,23 @@ def check_consistency(self): logging.warning(key + "=0, hence setting key update_num_comps " + "in group online automatically to False.") - def set(self, group, val_dict, set_if_not_exists=False, verbose=False): + def set(self, group:str, val_dict:dict, set_if_not_exists:bool=False, verbose=False) -> None: """ Add key-value pairs to a group. Existing key-value pairs will be overwritten if specified in val_dict, but not deleted. Args: - group: The name of the group. - val_dict: A dictionary with key-value pairs to be set for the group. - set_if_not_exists: Whether to set a key-value pair in a group if the key does not currently exist in the group. + group: The name of the group + val_dict: A dictionary with key-value pairs to be set for the group + warn_unused: + set_if_not_exists: Whether to set a key-value pair in a group if the key does not currently exist in the group. (DEPRECATED) """ + if set_if_not_exists: + logging.warning("The set_if_not_exists flag for CNMFParams.set() is deprecated and will be removed in a future version of Caiman") + # can't easily catch if it's passed but set to False, but that wouldn't do anything because of the default, + # and if they get that error it's at least really easy to fix - just remove the flag + # we don't want to support this because it makes the structure of the object unpredictable except at runtime + if not hasattr(self, group): raise KeyError(f'No group in CNMFParams named {group}') @@ -1002,7 +1009,7 @@ def set(self, group, val_dict, set_if_not_exists=False, verbose=False): if k not in d and not set_if_not_exists: if verbose: logging.warning( - f"NOT setting value of key {k} in group {group}, because no prior key existed...") + f"{group}/{k} not set: invalid target in CNMFParams object") else: try: if np.any(d[k] != v): @@ -1108,7 +1115,7 @@ def __repr__(self) -> str: return 'CNMFParams:\n\n' + '\n\n'.join(formatted_outputs) - def change_params(self, params_dict, allow_legacy=True, warn_unused=True, verbose=False) -> None: + def change_params(self, params_dict, allow_legacy:bool=True, warn_unused:bool=True, verbose:bool=False) -> None: """ Method for updating the params object by providing a dictionary. Args: @@ -1121,12 +1128,15 @@ def change_params(self, params_dict, allow_legacy=True, warn_unused=True, verbos were never used in populating the Params object. You really should not set this to False. Fix your code. """ - consumed = {} # Keep track of what parameters in params_dict were used to set something in params + # When we're ready to remove allow_legacy, this code will get a lot simpler + + consumed = {} # Keep track of what parameters in params_dict were used to set something in params (just for legacy API) for paramkey in params_dict: if paramkey in list(self.__dict__.keys()): # Proper pathed part cat_handle = getattr(self, paramkey) for k, v in params_dict[paramkey].items(): if k not in cat_handle and warn_unused: + # For regular/pathed API, we can notice right away if the user gave us something that won't update the object logging.warning(f"In setting CNMFParams, provided key {paramkey}/{k} was not consumed. This is a bug!") else: cat_handle[k] = v @@ -1143,7 +1153,7 @@ def change_params(self, params_dict, allow_legacy=True, warn_unused=True, verbos # END if warn_unused: for toplevel_k in params_dict: - if toplevel_k not in consumed and toplevel_k not in list(self.__dict__.keys()): + if toplevel_k not in consumed and toplevel_k not in list(self.__dict__.keys()): # When we remove legacy behaviour, this logic will simplify and fold into above logging.warning(f"In setting CNMFParams, provided toplevel key {toplevel_k} was unused. This is a bug!") self.check_consistency()