From 4e7a68d16321a3b337450964c42a23ba91c581f9 Mon Sep 17 00:00:00 2001
From: Jonathan Schmidt <54937397+JonathanSchmidt1@users.noreply.github.com>
Date: Thu, 2 May 2024 18:06:51 +0200
Subject: [PATCH] Fix `_get_dipole_info` for DDEC6 `ChargemolAnalysis` and add
 test case (#3801)

* fix _get_dipole_info for ddec6 and add test case

* refactor

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
---
 pymatgen/command_line/chargemol_caller.py   | 18 +++++++++---------
 tests/command_line/test_chargemol_caller.py |  1 +
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/pymatgen/command_line/chargemol_caller.py b/pymatgen/command_line/chargemol_caller.py
index 6ee292859b6..00cdb417e9a 100644
--- a/pymatgen/command_line/chargemol_caller.py
+++ b/pymatgen/command_line/chargemol_caller.py
@@ -416,8 +416,8 @@ def _get_dipole_info(filepath):
         idx = 0
         start = False
         dipoles = []
-        with open(filepath) as r:
-            for line in r:
+        with open(filepath) as file:
+            for line in file:
                 if "The following XYZ" in line:
                     start = True
                     idx += 1
@@ -425,7 +425,7 @@ def _get_dipole_info(filepath):
                 if start and line.strip() == "":
                     break
                 if idx >= 2:
-                    dipoles.append([float(d) for d in line.strip().split()[7:10]])
+                    dipoles.append(list(map(float, line.strip().split()[6:9])))
                 if start:
                     idx += 1
 
@@ -441,9 +441,9 @@ def _get_bond_order_info(filename):
         # Get where relevant info for each atom starts
         bond_order_info = {}
 
-        with open(filename, encoding="utf-8") as r:
+        with open(filename, encoding="utf-8") as file:
             start_idx = 0
-            for line in r:
+            for line in file:
                 split = line.strip().split()
                 if "Printing BOs" in line:
                     start_idx = int(split[5]) - 1
@@ -543,8 +543,8 @@ def _get_data_from_xyz(xyz_path) -> list[float]:
         """
         props = []
         if os.path.isfile(xyz_path):
-            with open(xyz_path) as r:
-                for idx, line in enumerate(r):
+            with open(xyz_path) as file:
+                for idx, line in enumerate(file):
                     if idx <= 1:
                         continue
                     if line.strip() == "":
@@ -568,8 +568,8 @@ def _get_cm5_data_from_output(ddec_analysis_path) -> list[float]:
         props = []
         if os.path.isfile(ddec_analysis_path):
             start = False
-            with open(ddec_analysis_path) as r:
-                for line in r:
+            with open(ddec_analysis_path) as file:
+                for line in file:
                     if "computed CM5" in line:
                         start = True
                         continue
diff --git a/tests/command_line/test_chargemol_caller.py b/tests/command_line/test_chargemol_caller.py
index a19ed8c392e..2c641f7c862 100644
--- a/tests/command_line/test_chargemol_caller.py
+++ b/tests/command_line/test_chargemol_caller.py
@@ -51,6 +51,7 @@ def test_parse_chargemol2(self):
         test_dir = f"{TEST_DIR}/spin_polarized"
         ca = ChargemolAnalysis(path=test_dir, run_chargemol=False)
         assert ca.ddec_spin_moments == [0.201595, 0.399203, 0.399203]
+        assert ca.dipoles == [[-0.0, 0.0, -0.127251], [0.0, -0.027857, -0.010944], [0.0, 0.027857, -0.010944]]
         assert ca.summary["ddec"]["bond_order_dict"][0]["bonded_to"][0]["spin_polarization"] == 0.0490
         assert ca.summary["ddec"]["spin_moments"] == ca.ddec_spin_moments
         assert ca.natoms is None