Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: The .as_dict() method on CrystalSystem is not Monty compatible #914

Open
Andrew-S-Rosen opened this issue Dec 23, 2023 · 6 comments · Fixed by #944
Open

Bug: The .as_dict() method on CrystalSystem is not Monty compatible #914

Andrew-S-Rosen opened this issue Dec 23, 2023 · 6 comments · Fixed by #944

Comments

@Andrew-S-Rosen
Copy link
Member

Problem

from emmet.core.symmetry import CrystalSystem
sym = CrystalSystem("Triclinic")
sym.as_dict()

This returns the string "Triclinic" rather than a dictionary. This would normally be fine, but the fact that it's a string means that doing dumpfn doesn't work on a data structure containing a CrystalSystem object.

from monty.serialization import dumpfn

dumpfn(sym, "test.json")

Traceback:

------------------------------------------------------------------------
TypeError                              Traceback (most recent call last)
Cell In[47], [line 1](vscode-notebook-cell:?execution_count=47&line=1)
----> [1](vscode-notebook-cell:?execution_count=47&line=1) dumpfn(CrystalSystem("Triclinic"),"test.json")

File [c:\Users\asros\miniconda\envs\zeolites\lib\site-packages\monty\serialization.py:123](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/serialization.py:123), in dumpfn(obj, fn, fmt, *args, **kwargs)
    [121](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/serialization.py:121)     if "cls" not in kwargs:
    [122](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/serialization.py:122)         kwargs["cls"] = MontyEncoder
--> [123](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/serialization.py:123)     fp.write(json.dumps(obj, *args, **kwargs))
    [124](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/serialization.py:124) else:
    [125](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/serialization.py:125)     raise TypeError(f"Invalid format: {fmt}")

File [c:\Users\asros\miniconda\envs\zeolites\lib\json\__init__.py:238](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/__init__.py:238), in dumps(obj, skipkeys, ensure_ascii, check_circular, allow_nan, cls, indent, separators, default, sort_keys, **kw)
    [232](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/__init__.py:232) if cls is None:
    [233](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/__init__.py:233)     cls = JSONEncoder
    [234](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/__init__.py:234) return cls(
    [235](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/__init__.py:235)     skipkeys=skipkeys, ensure_ascii=ensure_ascii,
    [236](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/__init__.py:236)     check_circular=check_circular, allow_nan=allow_nan, indent=indent,
    [237](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/__init__.py:237)     separators=separators, default=default, sort_keys=sort_keys,
--> [238](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/__init__.py:238)     **kw).encode(obj)

File [c:\Users\asros\miniconda\envs\zeolites\lib\json\encoder.py:199](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:199), in JSONEncoder.encode(self, o)
    [195](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:195)         return encode_basestring(o)
    [196](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:196) # This doesn't pass the iterator directly to ''.join() because the
    [197](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:197) # exceptions aren't as detailed.  The list call should be roughly
    [198](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:198) # equivalent to the PySequence_Fast that ''.join() would do.
--> [199](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:199) chunks = self.iterencode(o, _one_shot=True)
    [200](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:200) if not isinstance(chunks, (list, tuple)):
    [201](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:201)     chunks = list(chunks)

File [c:\Users\asros\miniconda\envs\zeolites\lib\json\encoder.py:257](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:257), in JSONEncoder.iterencode(self, o, _one_shot)
    [252](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:252) else:
    [253](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:253)     _iterencode = _make_iterencode(
    [254](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:254)         markers, self.default, _encoder, self.indent, floatstr,
    [255](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:255)         self.key_separator, self.item_separator, self.sort_keys,
    [256](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:256)         self.skipkeys, _one_shot)
--> [257](file:///C:/Users/asros/miniconda/envs/zeolites/lib/json/encoder.py:257) return _iterencode(o, 0)

File [c:\Users\asros\miniconda\envs\zeolites\lib\site-packages\monty\json.py:394](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/json.py:394), in MontyEncoder.default(self, o)
    [391](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/json.py:391)     d = o.as_dict()
    [393](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/json.py:393) if "@module" not in d:
--> [394](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/json.py:394)     d["@module"] = str(o.__class__.__module__)
    [395](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/json.py:395) if "@class" not in d:
    [396](file:///C:/Users/asros/miniconda/envs/zeolites/lib/site-packages/monty/json.py:396)     d["@class"] = str(o.__class__.__name__)

TypeError: 'str' object does not support item assignment

Proposed Solution

Modify the .as_dict() method?

Alternatives

No response

@Andrew-S-Rosen Andrew-S-Rosen changed the title Bug: The .as_dict() method on CrystalSystem is not Monty compatable Bug: The .as_dict() method on CrystalSystem is not Monty compatible Dec 23, 2023
@jmmshn
Copy link
Contributor

jmmshn commented Jan 24, 2024

Looks like I ran into something similar, resolved on monty's side:
materialsvirtuallab/monty#602

@Andrew-S-Rosen
Copy link
Member Author

Andrew-S-Rosen commented Jan 24, 2024

Oh that was smart! Thanks!! I'll check if is resolved with the monty update.

@Andrew-S-Rosen
Copy link
Member Author

Looks like your PR didn't fix this one btw, @jmmshn. I'll have to dig into it later.

@jmmshn
Copy link
Contributor

jmmshn commented Feb 21, 2024

So I think the custom as_dict for this takes precedence over the new general for the Enums

So something like this should work.

from monty.serialization import dumpfn
from enum import Enum
class Test(Enum):
    a = "a"
    b = "b"
t = Test.a
dumpfn(t, "test.json")

So I think if we just remove the custom as_dict and from_dict for that class then things should work. But we will have a see about the consequences.

@Andrew-S-Rosen
Copy link
Member Author

Andrew-S-Rosen commented Feb 21, 2024

Thanks for the pointer, @jmmshn! Removing the .as_dict() method in emmet.core.utils.ValueEnum results in an AttributeError:

AttributeError: 'CrystalSystem' object has no attribute 'as_dict'

I'll dig into this later in the week. I think I know where to look now. 👍

@Andrew-S-Rosen
Copy link
Member Author

@munrojm can you please re-open this? It won't let me do so. It needs to be reopened due to #974.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants