Skip to content

Commit

Permalink
Expanding coverage for generic_tags and commenting out unused/unreach…
Browse files Browse the repository at this point in the history 
…able code
  • Loading branch information
@benrich37
benrich37 committed Sep 25, 2024
1 parent 943f0c7 commit 8e21488
Show file tree
Hide file tree
Showing 2 changed files with 185 additions and 107 deletions.
212 changes: 107 additions & 105 deletions src/pymatgen/io/jdftx/generic_tags.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,10 @@
from abc import ABC, abstractmethod
from copy import deepcopy
from dataclasses import dataclass, field
from typing import TYPE_CHECKING, Any
from typing import Any

import numpy as np

if TYPE_CHECKING:
from pymatgen.core import Structure

__author__ = "Jacob Clary, Ben Rich"


Expand Down Expand Up @@ -982,32 +979,36 @@ def get_token_len(self) -> int:
min_token_len += subtag_token_len
return min_token_len

def check_representation(self, tag: str, value: Any) -> str:
"""Check the representation of the value.
Check the representation of the value.
Parameters
----------
tag : str
The tag to check the representation of the value for.
value : Any
The value to check the representation of.
Returns
-------
str
The representation of the value.
"""
if not self.allow_list_representation:
return "dict"
value_list = self.get_list_representation(tag, value)
value_dict = self.get_dict_representation(tag, value)
if value == value_list:
return "list"
if value == value_dict:
return "dict"
raise ValueError("Could not determine TagContainer representation, something is wrong")
# COMMENTING OUT THIS FUNCTION BREAKS NOTHING, KEEPING IT AS COMMENT FOR NOW
# TODO: get_list_representation gives something that get_dict_representation cannot parse, making this method
# only useful for asserting something is a dictionary. Figure out what list_representations are supposed to
# look like, or replace this function
# def check_representation(self, tag: str, value: Any) -> str:
# """Check the representation of the value.

# Check the representation of the value.

# Parameters
# ----------
# tag : str
# The tag to check the representation of the value for.
# value : Any
# The value to check the representation of.

# Returns
# -------
# str
# The representation of the value.
# """
# if not self.allow_list_representation:
# return "dict"
# value_list = self.get_list_representation(tag, value)
# value_dict = self.get_dict_representation(tag, value)
# if value == value_list:
# return "list"
# if value == value_dict:
# return "dict"
# raise ValueError("Could not determine TagContainer representation, something is wrong")

def _make_list(self, value: dict) -> list:
value_list = []
Expand Down Expand Up @@ -1074,8 +1075,7 @@ def get_list_representation(self, tag: str, value: Any) -> list:
if all(isinstance(entry, list) for entry in value):
return value # no conversion needed
if any(not isinstance(entry, dict) for entry in value):
raise ValueError(f"The {tag} tag set to {value} must be a list \
of dict")
raise ValueError(f"The {tag} tag set to {value} must be a list of dict")
tag_as_list = [self._make_list(entry) for entry in value]
else:
tag_as_list = self._make_list(value)
Expand Down Expand Up @@ -1163,76 +1163,77 @@ def get_dict_representation(self, tag: str, value: list) -> dict | list[dict]:
return self.read(tag, list_value)


@dataclass
class StructureDeferredTagContainer(TagContainer):
"""Class for tags that require a Pymatgen structure to process the value.
This tag class accommodates tags that can have complicated values that
depend on the number and species of atoms present. The species labels do
not necessarily have to be elements, but just match the species given in
the ion/ion-species tag(s). We will use the set of labels provided by the
ion tag(s) because that is a well-defined token, while it may not be
explicitly defined in ion-species.
Relevant tags: add-U, initial-magnetic-moments, initial-oxidation-states,
set-atomic-radius, setVDW
"""

defer_until_struc: bool = True

def read(self, tag: str, value: str, structure: Structure = None) -> None:
"""Read the value string for this tag.
Read the value string for this tag.
Parameters
----------
tag : str
The tag to read the value string for.
value : str
The value string to read.
structure : Structure, optional
The Pymatgen structure to use for reading the value string,
by default None.
"""
raise NotImplementedError

# """This method is similar to StrTag.read(), but with less validation
# because usually will
# get a string like 'Fe 2.0 2.5 Ni 1.0 1.1' as the value to process later

# If this method is called separately from the JDFTXInfile processing
# methods, a Pymatgen
# structure may be provided directly
# """
# try:
# value = str(value)
# except:
# raise ValueError(f"Could not set '{value}' to a str for {tag}!")

# if structure is not None:
# value = self.read_with_structure(tag, value, structure)
# return value

def read_with_structure(self, tag: str, value: str, structure: Structure) -> None:
"""Read tag/value pair with a Pymatgen structure provided.
Read tag/value pair with a Pymatgen structure provided.
Parameters
----------
tag : str
The tag to read the value string for.
value : str
The value string to read.
structure : Structure
The Pymatgen structure to use for reading the value string.
"""
raise NotImplementedError

# """Fully process the value string using data from the Pymatgen
# structure"""
# return self._TC_read(tag, value, structure)
# COMMENTING THIS OUT UNTIL IT IS MADE USABLE
# @dataclass
# class StructureDeferredTagContainer(TagContainer):
# """Class for tags that require a Pymatgen structure to process the value.

# This tag class accommodates tags that can have complicated values that
# depend on the number and species of atoms present. The species labels do
# not necessarily have to be elements, but just match the species given in
# the ion/ion-species tag(s). We will use the set of labels provided by the
# ion tag(s) because that is a well-defined token, while it may not be
# explicitly defined in ion-species.

# Relevant tags: add-U, initial-magnetic-moments, initial-oxidation-states,
# set-atomic-radius, setVDW
# """

# defer_until_struc: bool = True

# def read(self, tag: str, value: str, structure: Structure = None) -> None:
# """Read the value string for this tag.

# Read the value string for this tag.

# Parameters
# ----------
# tag : str
# The tag to read the value string for.
# value : str
# The value string to read.
# structure : Structure, optional
# The Pymatgen structure to use for reading the value string,
# by default None.
# """
# raise NotImplementedError

# # """This method is similar to StrTag.read(), but with less validation
# # because usually will
# # get a string like 'Fe 2.0 2.5 Ni 1.0 1.1' as the value to process later

# # If this method is called separately from the JDFTXInfile processing
# # methods, a Pymatgen
# # structure may be provided directly
# # """
# # try:
# # value = str(value)
# # except:
# # raise ValueError(f"Could not set '{value}' to a str for {tag}!")

# # if structure is not None:
# # value = self.read_with_structure(tag, value, structure)
# # return value

# def read_with_structure(self, tag: str, value: str, structure: Structure) -> None:
# """Read tag/value pair with a Pymatgen structure provided.

# Read tag/value pair with a Pymatgen structure provided.

# Parameters
# ----------
# tag : str
# The tag to read the value string for.
# value : str
# The value string to read.
# structure : Structure
# The Pymatgen structure to use for reading the value string.
# """
# raise NotImplementedError

# # """Fully process the value string using data from the Pymatgen
# # structure"""
# # return self._TC_read(tag, value, structure)


@dataclass
Expand Down Expand Up @@ -1500,9 +1501,10 @@ def read(self, tag: str, value_str: str) -> dict:
# reorder all tags to match order of __MASTER_TAG_LIST__ and do
# coarse-grained validation of read
subdict = {x: tempdict[x] for x in self.subtags if x in tempdict}
for subtag, subtag_type in self.subtags.items():
if not subtag_type.optional and subtag not in subdict:
raise ValueError(f"The {subtag} tag is not optional but was not populated during the read!")
# There are no forced subtags for dump
# for subtag, subtag_type in self.subtags.items():
# if not subtag_type.optional and subtag not in subdict:
# raise ValueError(f"The {subtag} tag is not optional but was not populated during the read!")
if len(value) > 0:
raise ValueError(
f"Something is wrong in the JDFTXInfile formatting, some values were not processed: {value}"
Expand Down
80 changes: 78 additions & 2 deletions tests/io/jdftx/test_generic_tags.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,17 @@

import pytest

from pymatgen.io.jdftx.generic_tags import AbstractTag, BoolTag, FloatTag, InitMagMomTag, IntTag, StrTag, TagContainer
from pymatgen.io.jdftx.jdftxinfile_master_format import get_tag_object
from pymatgen.io.jdftx.generic_tags import (
AbstractTag,
BoolTag,
BoolTagContainer,
FloatTag,
InitMagMomTag,
IntTag,
StrTag,
TagContainer,
)
from pymatgen.io.jdftx.jdftxinfile_master_format import get_dump_tag_container, get_tag_object


def test_abstract_tag():
Expand All @@ -14,6 +23,13 @@ def test_abstract_tag():
AbstractTag()


def test_stringify():
str_tag = StrTag(options=["ken"])
out_str = str(str_tag)
assert isinstance(out_str, str)
assert len(out_str)


def test_bool_tag():
bool_tag = BoolTag(write_value=False)
with pytest.raises(ValueError, match="Tag object has no get_list_representation method: barbie"):
Expand All @@ -27,6 +43,8 @@ def test_bool_tag():
bool_tag = BoolTag(write_value=True)
with pytest.raises(ValueError, match="Could not set 'not-appearing-in-read-TF-options' as True/False for barbie!"):
bool_tag.read("barbie", "not-appearing-in-read-TF-options")
with pytest.raises(TypeError):
bool_tag._validate_repeat("barbie", "ken")


class Unstringable:
Expand Down Expand Up @@ -125,6 +143,7 @@ def test_tagcontainer():
ValueError, match="Subtag allan is not allowed to repeat repeats in barbie's value allan 1 allan 2"
):
tagcontainer.read("barbie", "allan 1 allan 2")
###
tagcontainer = TagContainer(
can_repeat=False,
subtags={
Expand All @@ -137,6 +156,7 @@ def test_tagcontainer():
"barbie",
{"ken": "1", "allan": "barbie"}, # Raises a warning since barbie cannot be converted to int
)
###
tagcontainer = TagContainer(
can_repeat=True,
subtags={
Expand Down Expand Up @@ -179,6 +199,62 @@ def test_tagcontainer():
assert tagcontainer.get_token_len() == 3
with pytest.raises(TypeError):
tagcontainer.write("barbie", ["ken barbie"])
###
tagcontainer = get_tag_object("ion")
with pytest.warns(Warning):
tagcontainer.read("ion", "Fe 1 1 1 1 HyperPlane")
###
tagcontainer = TagContainer(
can_repeat=True,
allow_list_representation=False,
subtags={
"ken": StrTag(),
"allan": IntTag(),
},
)
strmatch = str([{"ken": "b"}, [["allan", 1]]])
with pytest.raises(
ValueError,
match=re.escape(f"barbie with {strmatch} cannot have nested lists/dicts mixed with bool/str/int/floats!"),
):
tagcontainer._check_for_mixed_nesting("barbie", [{"ken": "b"}, [["allan", 1]]])
strmatch = str([{"ken": "b"}, {"allan": 1}])
with pytest.raises(
ValueError,
match=re.escape(f"barbie with {strmatch} cannot have nested dicts mixed with bool/str/int/floats!"),
):
tagcontainer._check_for_mixed_nesting("barbie", [{"ken": "b"}, {"allan": 1}])
strmatch = str([["ken", "b"], ["allan", 1]])
with pytest.raises(
ValueError,
match=re.escape(f"barbie with {strmatch} cannot have nested lists mixed with bool/str/int/floats!"),
):
tagcontainer._check_for_mixed_nesting("barbie", [["ken", "b"], ["allan", 1]])


# def test_multiformattagcontainer():


def test_dumptagcontainer():
dtc = get_dump_tag_container()
with pytest.raises(
ValueError,
match=re.escape("Something is wrong in the JDFTXInfile formatting, some values were not processed: ['barbie']"),
):
dtc.read("dump", "barbie End DOS")


def test_booltagcontainer():
btc = BoolTagContainer(
subtags={
"ken": BoolTag(optional=False, write_tagname=True, write_value=False),
"allan": BoolTag(optional=True, write_tagname=True, write_value=False),
},
)
with pytest.raises(ValueError, match="The ken tag is not optional but was not populated during the read!"):
btc.read("barbie", "allan")
with pytest.raises(
ValueError,
match=re.escape("Something is wrong in the JDFTXInfile formatting, some values were not processed: ['aliah']"),
):
btc.read("barbie", "ken aliah")

0 comments on commit 8e21488

Please sign in to comment.