Skip to content

Commit

Permalink
Fixed formatting issues in code, and of output in write-structure step.
Browse files Browse the repository at this point in the history
  • Loading branch information
paulsaxe committed Jul 6, 2023
1 parent a35aa44 commit e7da6b9
Show file tree
Hide file tree
Showing 14 changed files with 94 additions and 54 deletions.
4 changes: 4 additions & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
History
=======

2023.7.6 -- Bugfixes
* Fixed output of number of structures written to SDF files, which was off by 1.
* Cleaned up the output for the write-structure step

2023.1.30 -- Fixed issue#43, duplicate systems or configuration created

* Reading a single structure from e.g. a .sdf file created a second system or
Expand Down
2 changes: 0 additions & 2 deletions read_structure_step/formats/mop/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,7 @@

@register_format_checker(".mop")
def check_format(file_name):

with open(file_name, "r") as f:

data = f.read()

if any(keyword in data for keyword in keywords):
Expand Down
3 changes: 0 additions & 3 deletions read_structure_step/formats/registries.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@


def register_reader(file_format):

tmp = file_format.split()
extension = tmp[0]
if extension[0] != ".":
Expand All @@ -52,7 +51,6 @@ def register_reader(file_format):
description = " ".join(tmp[1:])

def decorator_function(fn):

REGISTERED_READERS[extension] = {"function": fn, "description": description}

def wrapper_function(*args, **kwargs):
Expand Down Expand Up @@ -90,7 +88,6 @@ def wrapper_function(*args, **kwargs):

def register_format_checker(file_format):
def decorator_function(fn):

REGISTERED_FORMAT_CHECKERS[file_format] = fn

def wrapper_function(*args, **kwargs):
Expand Down
4 changes: 2 additions & 2 deletions read_structure_step/formats/sdf/sdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,8 +407,8 @@ def write_sdf(
t1 = time.time()
rate = structure_no / (t1 - t0)
printer(
f"Wrote {structure_no} structures in {t1 - t0:.1f} seconds = {rate:.2f} "
"per second"
f" Wrote {structure_no - 1} structures in {t1 - t0:.1f} seconds = "
f"{rate:.2f} per second"
)

if references:
Expand Down
1 change: 0 additions & 1 deletion read_structure_step/formats/xyz/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

@register_format_checker(".xyz")
def check_format(file_name):

with open(file_name, "r") as f:
lines = f.read().splitlines()

Expand Down
2 changes: 0 additions & 2 deletions read_structure_step/read.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,9 @@ def read(
file_name = os.path.abspath(file_name)

if extension is None:

extension = utils.guess_extension(file_name, use_file_name=True)

if extension is None:

extension = utils.guess_extension(file_name, use_file_name=False)

else:
Expand Down
5 changes: 3 additions & 2 deletions read_structure_step/read_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ def create_parser(self):
"""Setup the command-line / config file parser"""
# Need to mimic MOPAC step to find the MOPAC executable
parser_name = "mopac-step"
print(f"Parser {parser_name}, {self.step_type=}")
parser = getParser()

# Remember if the parser exists ... this type of step may have been
Expand Down Expand Up @@ -155,7 +154,9 @@ def description_text(self, P=None):
if file_type != "from extension":
extension = file_type.split()[0]
else:
if filename != "":
if self.is_expr(filename):
extension = "all"
elif filename != "":
path = PurePath(filename)
extension = path.suffix
if extension == ".gz":
Expand Down
1 change: 0 additions & 1 deletion read_structure_step/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ def guess_extension(file_name, use_file_name=False):
available_extensions = formats.registries.REGISTERED_FORMAT_CHECKERS.keys()

for extension in available_extensions:

extension_checker = formats.registries.REGISTERED_FORMAT_CHECKERS[extension]

if extension_checker(file_name) is True:
Expand Down
114 changes: 84 additions & 30 deletions read_structure_step/write_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import logging
from pathlib import PurePath
import textwrap

import read_structure_step
from .write import write
Expand Down Expand Up @@ -78,34 +79,76 @@ def description_text(self, P=None):
if not P:
P = self.parameters.values_to_dict()

text = f"Write structure to {P['file']}. "

# # What type of file?
# extension = ""
# filename = P["file"].strip()
# file_type = P["file type"]

# if self.is_expr(filename) or self.is_expr(file_type):
# extension = "all"
# else:
# if file_type != "from extension":
# extension = file_type.split()[0]
# else:
# if filename != "":
# path = PurePath(filename)
# extension = path.suffix
# if extension == ".gz":
# extension = path.stem.suffix
structures = P["structures"]
configs = P["configurations"]
ignore_missing = P["ignore missing"]
if isinstance(ignore_missing, bool):
if ignore_missing:
ignore_missing = "yes"
else:
ignore_missing = "no"

# # Get the metadata for the format
# metadata = get_format_metadata(extension)
n_per_file = P["number per file"]

# if extension == "all" or not metadata["single_structure"]:
# text += seamm.standard_parameters.multiple_structure_handling_description(P)
# else:
# text += seamm.standard_parameters.structure_handling_description(P)
if structures == "current configuration":
text = f"The current configuration will be written to {P['file']}. "
elif structures == "current system":
if configs == "all":
text = (
"All the configurations of the current system will be written to "
f"{P['file']}. "
)
else:
text = (
f"Configuration '{configs}' of the current system will be written "
f"to {P['file']}. "
)
if n_per_file != "all":
text += (
" The output will be broken into multiple files containing no "
f"more than {n_per_file} structures each."
)
elif structures == "all systems":
if configs == "all":
text = (
"All the configurations of all the systems will be written to "
f"{P['file']}. "
)
else:
text = (
f"Configuration '{configs}' of all systems will be written "
f"to {P['file']}. "
)
if ignore_missing == "yes":
pass
elif ignore_missing == "no":
text += (
"It will be an error if a system does not have the named "
"configuration."
)
else:
text += (
f"The value of {ignore_missing} will determine whether it is"
" an error if a system does not have the named configuration."
)
if n_per_file != "all":
text += (
" The output will be broken into multiple files containing no "
f"more than {n_per_file} structures each."
)
else:
text = (
f"The variable {structures} will determine which systems to write out, "
f"and {configs} the configurations for those systems."
)
if n_per_file != "all":
text += (
" The output will be broken into multiple files containing no "
f"more than {n_per_file} structures each."
)

return text
text = textwrap.fill(text, initial_indent=4 * " ", subsequent_indent=4 * " ")
return self.header + "\n" + text

def run(self):
"""Run a Write Structure step."""
Expand Down Expand Up @@ -134,7 +177,7 @@ def run(self):
)

# Print what we are doing
printer.important(__(self.description_text(P), indent=4 * " "))
printer.important(self.description_text(P))

# Write the file into the system
system_db = self.get_variable("_system_db")
Expand All @@ -145,16 +188,19 @@ def run(self):
errors = not P["ignore missing"]
configurations = []
if structures == "current configuration":
n_systems = 1
configurations.append(configuration)
elif structures == "current system":
n_systems = 1
if configs == "all":
for configuration in system.configurations:
configurations.append(configuration)
else:
cid = system.get_configuration_id(configs, errors=errors)
if cid is not None:
configurations.append(system.get_configuration(cid))
else:
cid = system.get_configuration_id(configs, errors=errors)
if cid is not None:
configurations.append(system.get_configuration(cid))
elif structures == "all systems":
n_systems = system_db.n_systems
if configs == "all":
for system in system_db.systems:
for configuration in system.configurations:
Expand All @@ -167,6 +213,14 @@ def run(self):

n_per_file = P["number per file"]
n_configurations = len(configurations)
if n_configurations > 1:
printer.important(
__(
f"\n Writing out {n_configurations} structures from {n_systems} "
"systems.",
indent=4 * " ",
)
)
if n_per_file == "all" or n_configurations <= n_per_file:
write(
filename,
Expand Down
3 changes: 1 addition & 2 deletions read_structure_step/write_structure_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ class WriteStructureParameters(seamm.Parameters):
"current configuration",
"current system",
"all systems",
"list of configurations",
),
"format_string": "s",
"description": "Structures to write:",
Expand All @@ -89,7 +88,7 @@ class WriteStructureParameters(seamm.Parameters):
"enumeration": ("all",),
"format_string": "s",
"description": "Configuration(s) to write:",
"help_text": "The configurations to write: a name, or 'all configurations'",
"help_text": "The configurations to write: a name, or 'all'",
},
"number per file": {
"default": "all",
Expand Down
1 change: 0 additions & 1 deletion tests/mopac_exists.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@


def mopac_exists():

if os.path.isdir("/opt/mopac/"):
mopac_path = "/opt/mopac/"
else:
Expand Down
2 changes: 0 additions & 2 deletions tests/test_formats.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ def configuration():
],
)
def test_format(configuration, structure):

file_name = build_filenames.build_data_filename(structure)
read_structure_step.read(file_name, configuration)

Expand Down Expand Up @@ -97,7 +96,6 @@ def test_format(configuration, structure):
reason="MOPAC could not be found",
)
def test_mopac(configuration):

file_name = build_filenames.build_data_filename("acetonitrile.mop")
read_structure_step.read(file_name, configuration)

Expand Down
4 changes: 0 additions & 4 deletions tests/test_read_structure_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,16 @@ def configuration():

@pytest.mark.parametrize("file_name", [1, [], {}, 1.0])
def test_read_filename_type(configuration, file_name):

with pytest.raises(TypeError):
read_structure_step.read(file_name, configuration)


def test_empty_filename(configuration):

with pytest.raises(NameError):
read_structure_step.read("", configuration)


def test_unregistered_reader(configuration):

with pytest.raises(KeyError):

xyz_file = build_filenames.build_data_filename("spc.xyz")
read_structure_step.read(xyz_file, configuration, extension=".mp3")
2 changes: 0 additions & 2 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ def configuration():
@pytest.mark.parametrize("file_name", ["spc.xyz", "spc"])
@pytest.mark.parametrize("extension", [None, ".xyz", "xyz", "XYZ", "xYz"])
def test_extensions(configuration, file_name, extension):

xyz_file = build_filenames.build_data_filename(file_name)
read_structure_step.read(xyz_file, configuration, extension=extension)

Expand All @@ -52,6 +51,5 @@ def test_extensions(configuration, file_name, extension):


def test_sanitize_file_format_regex_validation(configuration):

with pytest.raises(NameError):
read_structure_step.read("spc.xyz", configuration, extension=".xy-z")

0 comments on commit e7da6b9

Please sign in to comment.