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

Rename VaspInputSet.(potcar_functional->user_potcar_functional) and start testing DictSet #3035

Merged
merged 9 commits into from
Jun 3, 2023

Conversation

janosh
Copy link
Member

@janosh janosh commented Jun 2, 2023

Follow up to #3031 adding a test to make sure we don't regress again. I just realized we had 0 tests for DictSet before. While writing some, I noticed DictSet.as_dict() is also broken. Not only since #2972, even before due to not storing the validate_magmons init kwarg on instances.

VaspInputSet.potcar_functional was renamed to VaspInputSet.user_potcar_functional to fix DictSet.as_dict() which otherwise errors

NotImplementedError: Unable to automatically determine as_dict format from class. MSONAble requires all args to be present as either self.argname or self._argname

A DictSet @property was added aliasing .user_potcar_functional to .potcar_functional to avoid breaking changes.

@janosh janosh added io Input/output functionality fix Bug fix PRs vasp Vienna Ab initio Simulation Package labels Jun 2, 2023
@janosh janosh enabled auto-merge (squash) June 2, 2023 01:23
@janosh janosh changed the title Test gh 3031 Add class DictSetTest to start testing DictSet Jun 2, 2023
janosh added 3 commits June 1, 2023 18:36
…tional'

was renamed to user_potcar_functional
on pymatgen/io/vasp/sets.py:123
        if "mp" in extra_fields_single:
>           assert len(structs["mp"]) == len(extra_fields_single["mp"])
E           KeyError: 'mp'

pymatgen/ext/tests/test_optimade.py:54: KeyError
@janosh janosh changed the title Add class DictSetTest to start testing DictSet Breaking: Add class DictSetTest to start testing DictSet Jun 2, 2023
@janosh janosh added the breaking Breaking change label Jun 2, 2023
@janosh janosh disabled auto-merge June 2, 2023 01:43
@janosh janosh added the needs discussion Needs discussion to agree on actionable next steps label Jun 2, 2023
@janosh janosh changed the title Breaking: Add class DictSetTest to start testing DictSet Breaking: rename VaspInputSet.(potcar_functional->user_potcar_functional) and start testing DictSet Jun 2, 2023
@shyuep
Copy link
Member

shyuep commented Jun 3, 2023

Agreed.

@janosh janosh enabled auto-merge (squash) June 3, 2023 00:33
@janosh janosh disabled auto-merge June 3, 2023 01:11
@janosh janosh merged commit 051f853 into master Jun 3, 2023
@janosh janosh deleted the test-gh-3031 branch June 3, 2023 01:11
@janosh janosh changed the title Breaking: rename VaspInputSet.(potcar_functional->user_potcar_functional) and start testing DictSet Rename VaspInputSet.(potcar_functional->user_potcar_functional) and start testing DictSet Jun 3, 2023
@janosh janosh removed needs discussion Needs discussion to agree on actionable next steps breaking Breaking change labels Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix PRs io Input/output functionality vasp Vienna Ab initio Simulation Package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants