Skip to content

Commit

Permalink
Fix #599 by updating error message
Browse files Browse the repository at this point in the history
- No longer use length of dummy value
- Apply concentration validator before chemical_symbols which may be
  invalid
  • Loading branch information
ml-evs committed Oct 19, 2020
1 parent e7432a2 commit 8bbe1b1
Showing 1 changed file with 25 additions and 13 deletions.
38 changes: 25 additions & 13 deletions optimade/models/structures.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,12 @@ def validate_chemical_symbols(cls, v):
raise ValueError(f"{v} MUST be in {EXTENDED_CHEMICAL_SYMBOLS}")
return v

@validator("concentration")
@validator("concentration", pre=True)
def validate_concentration(cls, v, values):
if len(v) != len(values.get("chemical_symbols", [])):
raise ValueError(
f"Length of concentration ({len(v)}) MUST equal length of chemical_symbols ({len(values.get('chemical_symbols', 'Not specified'))})"
f"Length of concentration ({len(v)}) MUST equal length of chemical_symbols "
f"({len(values.get('chemical_symbols', []))})"
)
return v

Expand Down Expand Up @@ -227,7 +228,8 @@ def validate_sites_in_groups(cls, v):
def check_self_consistency(cls, v, values):
if len(v) != len(values.get("sites_in_groups", [])):
raise ValueError(
f"sites_in_groups and group_probabilities MUST be of same length, but are {len(values.get('sites_in_groups', 'Not specified'))} and {len(v)}, respectively"
f"sites_in_groups and group_probabilities MUST be of same length, "
f"but are {len(values.get('sites_in_groups', []))} and {len(v)}, respectively"
)
return v

Expand Down Expand Up @@ -825,7 +827,8 @@ def null_values_for_whole_vector(cls, v):
def validate_nsites(cls, v, values):
if v != len(values.get("cartesian_site_positions", [])):
raise ValueError(
f"nsites (value: {v}) MUST equal length of cartesian_site_positions (value: {len(values.get('cartesian_site_positions', []))})"
f"nsites (value: {v}) MUST equal length of cartesian_site_positions "
f"(value: {len(values.get('cartesian_site_positions', []))})"
)
return v

Expand All @@ -837,7 +840,8 @@ def validate_species_at_sites(cls, v, values):
)
if len(v) != values.get("nsites", 0):
raise ValueError(
f"Number of species_at_sites (value: {len(v)}) MUST equal number of sites (value: {values.get('nsites', 'Not specified')})"
f"Number of species_at_sites (value: {len(v)}) MUST equal number of sites "
f"(value: {values.get('nsites', 'Not specified')})"
)
all_species_names = {
getattr(_, "name", None) for _ in values.get("species", [{}])
Expand All @@ -846,7 +850,8 @@ def validate_species_at_sites(cls, v, values):
for value in v:
if value not in all_species_names:
raise ValueError(
f"species_at_sites MUST be represented by a species' name, but {value} was not found in the list of species names: {all_species_names}"
"species_at_sites MUST be represented by a species' name, "
f"but {value} was not found in the list of species names: {all_species_names}"
)
return v

Expand All @@ -871,13 +876,15 @@ def validate_structure_features(cls, v, values):
if len(species.chemical_symbols) > 1:
if StructureFeatures.DISORDER not in v:
raise ValueError(
f"{StructureFeatures.DISORDER.value} MUST be present when any one entry in species has a chemical_symbols list greater than one element"
f"{StructureFeatures.DISORDER.value} MUST be present when any one entry in species "
"has a chemical_symbols list greater than one element"
)
break
else:
if StructureFeatures.DISORDER in v:
raise ValueError(
f"{StructureFeatures.DISORDER.value} MUST NOT be present, since all species' chemical_symbols lists are equal to or less than one element"
f"{StructureFeatures.DISORDER.value} MUST NOT be present, since all species' chemical_symbols "
"lists are equal to or less than one element"
)
# assemblies
if values.get("assemblies", None) is not None:
Expand All @@ -888,7 +895,8 @@ def validate_structure_features(cls, v, values):
else:
if StructureFeatures.ASSEMBLIES in v:
raise ValueError(
f"{StructureFeatures.ASSEMBLIES.value} MUST NOT be present, since the property of the same name is not present"
f"{StructureFeatures.ASSEMBLIES.value} MUST NOT be present, "
"since the property of the same name is not present"
)
# site_attachments
for species in values.get("species", []):
Expand All @@ -897,27 +905,31 @@ def validate_structure_features(cls, v, values):
if getattr(species, "attached", None) is not None:
if StructureFeatures.SITE_ATTACHMENTS not in v:
raise ValueError(
f"{StructureFeatures.SITE_ATTACHMENTS.value} MUST be present when any one entry in species includes attached and nattached"
f"{StructureFeatures.SITE_ATTACHMENTS.value} MUST be present when any one entry "
"in species includes attached and nattached"
)
break
else:
if StructureFeatures.SITE_ATTACHMENTS in v:
raise ValueError(
f"{StructureFeatures.SITE_ATTACHMENTS.value} MUST NOT be present, since no species includes the attached and nattached fields"
f"{StructureFeatures.SITE_ATTACHMENTS.value} MUST NOT be present, since no species includes "
"the attached and nattached fields"
)
# implicit_atoms
species_names = [_.name for _ in values.get("species", [])]
for name in species_names:
if name not in values.get("species_at_sites", []):
if StructureFeatures.IMPLICIT_ATOMS not in v:
raise ValueError(
f"{StructureFeatures.IMPLICIT_ATOMS.value} MUST be present when any one entry in species is not represented in species_at_sites"
f"{StructureFeatures.IMPLICIT_ATOMS.value} MUST be present when any one entry in species "
"is not represented in species_at_sites"
)
break
else:
if StructureFeatures.IMPLICIT_ATOMS in v:
raise ValueError(
f"{StructureFeatures.IMPLICIT_ATOMS.value} MUST NOT be present, since all species are represented in species_at_sites"
f"{StructureFeatures.IMPLICIT_ATOMS.value} MUST NOT be present, since all species are "
"represented in species_at_sites"
)
return v

Expand Down

0 comments on commit 8bbe1b1

Please sign in to comment.