Recover commented out code in tests and mark with pytest.mark.skip …

…instead (#4027)

* recover some commented out test unit

* replace legacy random generator

* recover more commented out test

* avoid code-like comment in case we want to enable ERA someday

tests/analysis/chemenv/coordination_environments/test_coordination_geometry_finder.py

@@ -1,10 +1,17 @@
 from __future__ import annotations
 
+import json
+import os
+
 import numpy as np
 import pytest
 from numpy.testing import assert_allclose
 from pytest import approx
 
+from pymatgen.analysis.chemenv.coordination_environments.chemenv_strategies import (
+    SimpleAbundanceChemenvStrategy,
+    SimplestChemenvStrategy,
+)
 from pymatgen.analysis.chemenv.coordination_environments.coordination_geometries import AllCoordinationGeometries
 from pymatgen.analysis.chemenv.coordination_environments.coordination_geometry_finder import (
     AbstractGeometry,
@@ -27,7 +34,7 @@ def setUp(self):
             structure_refinement=self.lgf.STRUCTURE_REFINEMENT_NONE,
         )
 
-    #     self.strategies = [SimplestChemenvStrategy(), SimpleAbundanceChemenvStrategy()]
+    # self.strategies = [SimplestChemenvStrategy(), SimpleAbundanceChemenvStrategy()]
 
     def test_abstract_geometry(self):
         cg_ts3 = self.lgf.allcg["TS:3"]
@@ -117,45 +124,47 @@ def test_abstract_geometry(self):
         for perm_csm_dict in permutations_symmetry_measures:
             assert perm_csm_dict["symmetry_measure"] == approx(0.140355832317)
 
-    # def _strategy_test(self, strategy):
-    #     files = []
-    #     for _dirpath, _dirnames, filenames in os.walk(json_dir):
-    #         files.extend(filenames)
-    #         break
-
-    #     for _ifile, json_file in enumerate(files):
-    #         with self.subTest(json_file=json_file):
-    #             with open(f"{json_dir}/{json_file}") as file:
-    #                 dct = json.load(file)
-
-    #             atom_indices = dct["atom_indices"]
-    #             expected_geoms = dct["expected_geoms"]
-
-    #             struct = Structure.from_dict(dct["structure"])
-
-    #             struct = self.lgf.setup_structure(struct)
-    #             se = self.lgf.compute_structure_environments_detailed_voronoi(
-    #                 only_indices=atom_indices, maximum_distance_factor=1.5
-    #             )
-
-    #             # All strategies should get the correct environment with their default parameters
-    #             strategy.set_structure_environments(se)
-    #             for ienv, isite in enumerate(atom_indices):
-    #                 ce = strategy.get_site_coordination_environment(struct[isite])
-    #                 try:
-    #                     coord_env = ce[0]
-    #                 except TypeError:
-    #                     coord_env = ce
-    #                 # Check that the environment found is the expected one
-    #                 assert coord_env == expected_geoms[ienv]
-
-    # def test_simplest_chemenv_strategy(self):
-    #     strategy = SimplestChemenvStrategy()
-    #     self._strategy_test(strategy)
-
-    # def test_simple_abundance_chemenv_strategy(self):
-    #     strategy = SimpleAbundanceChemenvStrategy()
-    #     self._strategy_test(strategy)
+    def _strategy_test(self, strategy):
+        files = []
+        for _dirpath, _dirnames, filenames in os.walk(json_dir):
+            files.extend(filenames)
+            break
+
+        for _ifile, json_file in enumerate(files):
+            with self.subTest(json_file=json_file):
+                with open(f"{json_dir}/{json_file}") as file:
+                    dct = json.load(file)
+
+                atom_indices = dct["atom_indices"]
+                expected_geoms = dct["expected_geoms"]
+
+                struct = Structure.from_dict(dct["structure"])
+
+                struct = self.lgf.setup_structure(struct)
+                se = self.lgf.compute_structure_environments_detailed_voronoi(
+                    only_indices=atom_indices, maximum_distance_factor=1.5
+                )
+
+                # All strategies should get the correct environment with their default parameters
+                strategy.set_structure_environments(se)
+                for ienv, isite in enumerate(atom_indices):
+                    ce = strategy.get_site_coordination_environment(struct[isite])
+                    try:
+                        coord_env = ce[0]
+                    except TypeError:
+                        coord_env = ce
+                    # Check that the environment found is the expected one
+                    assert coord_env == expected_geoms[ienv]
+
+    @pytest.mark.skip("TODO: need someone to fix this")
+    def test_simplest_chemenv_strategy(self):
+        strategy = SimplestChemenvStrategy()
+        self._strategy_test(strategy)
+
+    @pytest.mark.skip("TODO: need someone to fix this")
+    def test_simple_abundance_chemenv_strategy(self):
+        strategy = SimpleAbundanceChemenvStrategy()
+        self._strategy_test(strategy)
 
     def test_perfect_environments(self):
         allcg = AllCoordinationGeometries()

tests/analysis/test_molecule_structure_comparator.py

@@ -2,8 +2,11 @@
 
 from unittest import TestCase
 
+import pytest
+
 from pymatgen.analysis.molecule_structure_comparator import MoleculeStructureComparator
 from pymatgen.core.structure import Molecule
+from pymatgen.io.qchem.outputs import QCOutput
 from pymatgen.util.testing import TEST_FILES_DIR
 
 __author__ = "xiaohuiqu"
@@ -129,14 +132,15 @@ def test_to_and_from_dict(self):
         d2 = MoleculeStructureComparator.from_dict(d1).as_dict()
         assert d1 == d2
 
-    # def test_structural_change_in_geom_opt(self):
-    #     qcout_path = f"{TEST_DIR}/mol_1_3_bond.qcout"
-    #     qcout = QcOutput(qcout_path)
-    #     mol1 = qcout.data[0]["molecules"][0]
-    #     mol2 = qcout.data[0]["molecules"][-1]
-    #     priority_bonds = [[0, 1], [0, 2], [1, 3], [1, 4], [1, 7], [2, 5], [2, 6], [2, 8], [4, 6], [4, 10], [6, 9]]
-    #     msc = MoleculeStructureComparator(priority_bonds=priority_bonds)
-    #     assert msc.are_equal(mol1, mol2)
+    @pytest.mark.skip("TODO: need someone to fix this")
+    def test_structural_change_in_geom_opt(self):
+        qcout_path = f"{TEST_DIR}/mol_1_3_bond.qcout"
+        qcout = QCOutput(qcout_path)
+        mol1 = qcout.data[0]["molecules"][0]
+        mol2 = qcout.data[0]["molecules"][-1]
+        priority_bonds = [[0, 1], [0, 2], [1, 3], [1, 4], [1, 7], [2, 5], [2, 6], [2, 8], [4, 6], [4, 10], [6, 9]]
+        msc = MoleculeStructureComparator(priority_bonds=priority_bonds)
+        assert msc.are_equal(mol1, mol2)
 
     def test_get_13_bonds(self):
         priority_bonds = [

tests/analysis/test_surface_analysis.py

@@ -2,6 +2,7 @@
 
 import json
 
+import pytest
 from numpy.testing import assert_allclose
 from pytest import approx
 from sympy import Number, Symbol
@@ -248,41 +249,43 @@ def test_entry_dict_from_list(self):
         surf_ene_plotter = SurfaceEnergyPlotter(all_Pt_slab_entries, self.Pt_analyzer.ucell_entry)
         assert surf_ene_plotter.list_of_chempots == self.Pt_analyzer.list_of_chempots
 
-    # def test_monolayer_vs_BE(self):
-    #     for el in self.Oads_analyzer_dict:
-    #         # Test WulffShape for adsorbed surfaces
-    #         analyzer = self.Oads_analyzer_dict[el]
-    #         plt = analyzer.monolayer_vs_BE()
-    #
-    # def test_area_frac_vs_chempot_plot(self):
-    #
-    #     for el in self.Oads_analyzer_dict:
-    #         # Test WulffShape for adsorbed surfaces
-    #         analyzer = self.Oads_analyzer_dict[el]
-    #         plt = analyzer.area_frac_vs_chempot_plot(x_is_u_ads=True)
-    #
-    # def test_chempot_vs_gamma_clean(self):
-    #
-    #     plt = self.Cu_analyzer.chempot_vs_gamma_clean()
-    #     for el in self.Oads_analyzer_dict:
-    #         # Test WulffShape for adsorbed surfaces
-    #         analyzer = self.Oads_analyzer_dict[el]
-    #         plt = analyzer.chempot_vs_gamma_clean(x_is_u_ads=True)
-    #
-    # def test_chempot_vs_gamma_facet(self):
-    #
-    #     for el, val in self.metals_O_entry_dict.items():
-    #         for hkl in val:
-    #             # Test WulffShape for adsorbed surfaces
-    #             analyzer = self.Oads_analyzer_dict[el]
-    #             plt = analyzer.chempot_vs_gamma_facet(hkl)
-    # def test_surface_chempot_range_map(self):
-    #
-    #     for el, val in self.metals_O_entry_dict.items():
-    #         for hkl in val:
-    #             # Test WulffShape for adsorbed surfaces
-    #             analyzer = self.Oads_analyzer_dict[el]
-    #             plt = analyzer.chempot_vs_gamma_facet(hkl)
+    @pytest.mark.skip("TODO: need someone to fix this")
+    def test_monolayer_vs_BE(self):
+        for el in self.Oads_analyzer_dict:
+            # Test WulffShape for adsorbed surfaces
+            analyzer = self.Oads_analyzer_dict[el]
+            analyzer.monolayer_vs_BE()
+
+    @pytest.mark.skip("TODO: need someone to fix this")
+    def test_area_frac_vs_chempot_plot(self):
+        for el in self.Oads_analyzer_dict:
+            # Test WulffShape for adsorbed surfaces
+            analyzer = self.Oads_analyzer_dict[el]
+            analyzer.area_frac_vs_chempot_plot(x_is_u_ads=True)
+
+    @pytest.mark.skip("TODO: need someone to fix this")
+    def test_chempot_vs_gamma_clean(self):
+        self.Cu_analyzer.chempot_vs_gamma_clean()
+        for el in self.Oads_analyzer_dict:
+            # Test WulffShape for adsorbed surfaces
+            analyzer = self.Oads_analyzer_dict[el]
+            analyzer.chempot_vs_gamma_clean(x_is_u_ads=True)
+
+    @pytest.mark.skip("TODO: need someone to fix this")
+    def test_chempot_vs_gamma_facet(self):
+        for el, val in self.metals_O_entry_dict.items():
+            for hkl in val:
+                # Test WulffShape for adsorbed surfaces
+                analyzer = self.Oads_analyzer_dict[el]
+                analyzer.chempot_vs_gamma_facet(hkl)
+
+    @pytest.mark.skip("TODO: need someone to fix this")
+    def test_surface_chempot_range_map(self):
+        for el, val in self.metals_O_entry_dict.items():
+            for hkl in val:
+                # Test WulffShape for adsorbed surfaces
+                analyzer = self.Oads_analyzer_dict[el]
+                analyzer.chempot_vs_gamma_facet(hkl)
 
 
 class TestWorkFunctionAnalyzer(PymatgenTest):

tests/command_line/test_critic2_caller.py

@@ -90,6 +90,22 @@ def setUp(self):
         self.c2o_new_format = Critic2Analysis(structure, reference_stdout_new_format)
 
     def test_to_from_dict(self):
+        """
+        reference dictionary for c2o.critical_points[0].as_dict()
+        {'@class': 'CriticalPoint',
+         '@module': 'pymatgen.command_line.critic2_caller',
+         'coords': None,
+         'field': 93848.0413,
+         'field_gradient': 0.0,
+         'field_hessian': [[-2593274446000.0, -3.873587547e-19, -1.704530713e-08],
+                           [-3.873587547e-19, -2593274446000.0, 1.386877485e-18],
+                           [-1.704530713e-08, 1.386877485e-18, -2593274446000.0]],
+         'frac_coords': [0.333333, 0.666667, 0.213295],
+         'index': 0,
+         'multiplicity': 1.0,
+         'point_group': 'D3h',
+         'type': < CriticalPointType.nucleus: 'nucleus' >}
+        """
         assert len(self.c2o.critical_points) == 6
         assert len(self.c2o.nodes) == 14
         assert len(self.c2o.edges) == 10
@@ -98,21 +114,6 @@ def test_to_from_dict(self):
         assert len(self.c2o_new_format.nodes) == 14
         assert len(self.c2o_new_format.edges) == 10
 
-        # reference dictionary for c2o.critical_points[0].as_dict()
-        # {'@class': 'CriticalPoint',
-        #  '@module': 'pymatgen.command_line.critic2_caller',
-        #  'coords': None,
-        #  'field': 93848.0413,
-        #  'field_gradient': 0.0,
-        #  'field_hessian': [[-2593274446000.0, -3.873587547e-19, -1.704530713e-08],
-        #                    [-3.873587547e-19, -2593274446000.0, 1.386877485e-18],
-        #                    [-1.704530713e-08, 1.386877485e-18, -2593274446000.0]],
-        #  'frac_coords': [0.333333, 0.666667, 0.213295],
-        #  'index': 0,
-        #  'multiplicity': 1.0,
-        #  'point_group': 'D3h',
-        #  'type': < CriticalPointType.nucleus: 'nucleus' >}
-
         assert str(self.c2o.critical_points[0].type) == "CriticalPointType.nucleus"
 
         # test connectivity