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]: 602 Enum serialization was a breaking change #610

Closed
4 tasks done
janosh opened this issue Jan 28, 2024 · 5 comments
Closed
4 tasks done

[Bug]: 602 Enum serialization was a breaking change #610

janosh opened this issue Jan 28, 2024 · 5 comments
Labels

Comments

@janosh
Copy link
Contributor

janosh commented Jan 28, 2024

Email (Optional)

No response

Version

2024.1.26

Which OS(es) are you using?

  • MacOS
  • Windows
  • Linux

What happened?

TestPhaseDiagram.test_read_json started failing after materialsproject/pymatgen@5137111 bumping monty to 2024.1.26.

Code snippet

def test_read_json(self):
    dumpfn(self.pd, f"{self.tmp_path}/pd.json")
    pd = loadfn(f"{self.tmp_path}/pd.json")
    assert isinstance(pd, PhaseDiagram)
    assert {*pd.as_dict()} == {*self.pd.as_dict()}

Log output

monty/json.py:517: in process_decoded
    return cls_.from_dict(data)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

d = {'value': 'O'}

    @staticmethod
    def from_dict(d) -> Element:
        """Makes Element obey the general json interface used in pymatgen for
        easier serialization.
        """
>       return Element(d["element"])
E       KeyError: 'element'

/Users/janosh/dev/pmg/pymatgen/core/periodic_table.py:755: KeyError

Code of Conduct

  • I agree to follow this project's Code of Conduct
@janosh janosh added the bug label Jan 28, 2024
janosh added a commit to esoteric-ephemera/pymatgen that referenced this issue Jan 28, 2024
janosh added a commit to materialsproject/pymatgen that referenced this issue Jan 28, 2024
* Fix Vasprun Potcar search in cases where no leading  is specified

* change potcar scrambling to not affect titel, update test files

* Update  and test comments to cite issue

* fix tests

* fix str | Path | bool type anno, remove debug print

* requirements.txt fix bad merge conflict resolution

* maybe fix failing TestPhaseDiagram.test_read_json

* revert monty==2024.1.23

see materialsvirtuallab/monty#610

* setup.py monty<=2024.1.26

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
@Andrew-S-Rosen
Copy link
Contributor

CC @jmmshn

@jmmshn
Copy link
Contributor

jmmshn commented Jan 29, 2024

Thansk for pining me @Andrew-S-Rosen !
So it looks like serialization should respect custom as_dict and from_dicts but there new code was inserted into the wrong place.
Fixing now.

@jmmshn
Copy link
Contributor

jmmshn commented Jan 29, 2024

The same pymatgen test now passes on my local.

@Andrew-S-Rosen
Copy link
Contributor

@janosh This can now be closed.

@janosh
Copy link
Contributor Author

janosh commented Jan 29, 2024

thanks for the quick fix @jmmshn! 👍

@janosh janosh closed this as completed Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants