Fixed code formatting and output formatting

HISTORY.rst

@@ -2,6 +2,15 @@
 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
+    configuration, depending on the stucture control options.
+
 2023.1.24 -- Added handler for XYZ files and added properties
 
   * Added a custom handler for XYZ files to cope with some of the variant formats.

read_structure_step/formats/mop/__init__.py

@@ -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):

read_structure_step/formats/registries.py

@@ -38,7 +38,6 @@
 
 
 def register_reader(file_format):
-
     tmp = file_format.split()
     extension = tmp[0]
     if extension[0] != ".":
@@ -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):
@@ -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):

read_structure_step/formats/sdf/sdf.py

@@ -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:

read_structure_step/formats/xyz/__init__.py

@@ -4,7 +4,6 @@
 
 @register_format_checker(".xyz")
 def check_format(file_name):
-
     with open(file_name, "r") as f:
         lines = f.read().splitlines()
 

read_structure_step/read.py

@@ -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:

read_structure_step/read_structure.py

@@ -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
@@ -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":
@@ -234,9 +235,8 @@ def run(self):
             )
 
             # Finish the output
-            system, configuration = self.get_system_configuration(
-                P, structure_handling=False
-            )
+            system, configuration = self.get_system_configuration()
+
             if configurations is None or len(configurations) == 1:
                 if configuration.periodicity == 3:
                     space_group = configuration.symmetry.group

read_structure_step/utils.py

@@ -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:

read_structure_step/write_structure.py

@@ -13,6 +13,7 @@
 
 import logging
 from pathlib import PurePath
+import textwrap
 
 import read_structure_step
 from .write import write
@@ -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."""
@@ -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")
@@ -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:
@@ -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,

read_structure_step/write_structure_parameters.py

@@ -76,7 +76,6 @@ class WriteStructureParameters(seamm.Parameters):
                 "current configuration",
                 "current system",
                 "all systems",
-                "list of configurations",
             ),
             "format_string": "s",
             "description": "Structures to write:",
@@ -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",

tests/mopac_exists.py

@@ -2,7 +2,6 @@
 
 
 def mopac_exists():
-
     if os.path.isdir("/opt/mopac/"):
         mopac_path = "/opt/mopac/"
     else:

tests/test_formats.py

@@ -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)
 
@@ -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)
 

tests/test_read_structure_step.py

@@ -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")

tests/test_utils.py

@@ -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)
 
@@ -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")