jsanitize does not use .as_dict() on MSONables unless strict=True
Andrew-S-Rosen opened this issue · 3 comments
When using monty.json.jsanitize
, it does not use the .as_dict()
method on MSONable
objects unless strict=True
.
While one can use strict=True
to force the MSONable
serialization, there are many cases where the user may want the input object to be converted to a str
(rather than having an exception thrown) if it's of a type that jsanitize does not "understand" while still benefiting from the .as_dict()
conversion of MSONable
s. After all, the docstring does say "also recursively encodes all objects using Monty's as_dict()
protocol," which is not really the case unless you force strict=True
.
It can be traced back to this line:
Lines 502 to 503 in 34493ab
Here is an example:
from pymatgen.core import Structure
from monty.json import jsanitize
struct = Structure(
lattice=[[0, 2.13, 2.13], [2.13, 0, 2.13], [2.13, 2.13, 0]],
species=["Mg", "O"],
coords=[[0, 0, 0], [0.5, 0.5, 0.5]],
)
jsanitize(struct)
'Full Formula (Mg1 O1)\nReduced Formula: MgO\nabc : 3.012275 3.012275 3.012275\nangles: 60.000000 60.000000 60.000000\nSites (2)\n # SP a b c\n--- ---- --- --- ---\n
0 Mg 0 0 0\n 1 O 0.5 0.5 0.5'
If the desire is to maintain backwards-compatibility, one option is to create an allow_msonable
kwarg and introduce the following block of code before the conversion to str
:
if allow_msonable and isinstance(obj, MSONable):
return obj.as_dict()
However, if this is considered to be a bug (unclear based on the description), then the allow_msonable
kwarg isn't needed.
@shyuep, what are your thoughts on having jsanitize
return MSONable
objects via .as_dict()
rather than str
by default? Was there a specific reason why this was avoided in the current implementation?
I think having a recurse_msonable
would be useful. I think the reason why str is the default is to avoid a potentially expensive expansion of MSONAble objects when it is not needed.
Closed in #350.