Alright, I just had some issues with this class, due to what, I think, is rather unintuitive behavior. Raising this issue here, in the hope it can lead to API improvements.
I obtained an instance of the PwSettingsDict via self.parameters from a koopmans.calculators._pw.PWCalculator class instance. First, it took me quite some time to figure out that I'm, in fact, not dealing with a dictionary, but an instance of the PwSettingsDict class. This is because the result of printing the instance looks exactly the same as it would for a dictionary. So I propose to wrap the output for example:
def __str__(self):
return f"<PwSettingsDict: {self.data}>"
so that
In[1]: print(settingsdict_instance)
Out[1]: <PwSettingsDict: {actual-contents}>
or something similar, possibly also at other stages of the inheritance hierarchy (SettingsDict, UserDict).
The second behavior that was different from the way I was expecting the method to behave was the fact that todict() doesn't just return a 1:1 dictionary representation of the (_collections_abc.MutableMapping) SettingsDict, but instead does some internal validation/filtering. While on the other hand, the fromdict classmethod just parses the dictionary 1:1. So I propose to add an optional argument to the todict() method, which can be used to return the full dictionary. I would expect the default behavior to be that I can convert with todict()/fromdict() arbitrarily without changing the contents of the object.
Alright, I just had some issues with this class, due to what, I think, is rather unintuitive behavior. Raising this issue here, in the hope it can lead to API improvements.
I obtained an instance of the
PwSettingsDictviaself.parametersfrom akoopmans.calculators._pw.PWCalculatorclass instance. First, it took me quite some time to figure out that I'm, in fact, not dealing with a dictionary, but an instance of thePwSettingsDictclass. This is because the result of printing the instance looks exactly the same as it would for a dictionary. So I propose to wrap the output for example:so that
or something similar, possibly also at other stages of the inheritance hierarchy (
SettingsDict,UserDict).The second behavior that was different from the way I was expecting the method to behave was the fact that
todict()doesn't just return a 1:1 dictionary representation of the (_collections_abc.MutableMapping)SettingsDict, but instead does some internal validation/filtering. While on the other hand, thefromdictclassmethodjust parses the dictionary 1:1. So I propose to add an optional argument to thetodict()method, which can be used to return the full dictionary. I would expect the default behavior to be that I can convert withtodict()/fromdict()arbitrarily without changing the contents of the object.