Code cleanup

finalcif/appwindow.py

@@ -1312,6 +1312,7 @@ def load_changes_cif(self) -> bool:
             if item.pair is not None:
                 key, value = item.pair
                 value = gemmi.cif.as_string(value).strip()
+                self.add_row(key=key, value=value)
                 self.ui.cif_main_table.setText(key=key, column=Column.EDIT, color=None, txt=value)
         for loop in changes.loops:
             self.cif.add_loop_to_cif(loop_tags=loop.tags, loop_values=loop.values)
@@ -1498,7 +1499,7 @@ def _load_block(self, index: int, load_changes: bool = True) -> None:
             if self.changes_answer == QMessageBox.Yes:
                 try:
                     self.load_changes_cif()
-                except Exception as e:
+                except GemmiError as e:
                     unable_to_open_message(parent=self, filepath=self.finalcif_changes_filename, not_ok=e)
             if self.running_inside_unit_test and changes_exist:
                 self.load_changes_cif()

finalcif/gui/import_selector.py

@@ -34,7 +34,7 @@ def _connect_signals_and_slots(self) -> None:
         self.ui.importSelectedPushbutton.clicked.connect(self.import_key_loop)
 
     def import_key_loop(self) -> None:
-        self.import_clicked.emit(self.get_keys_to_import(), self.get_loops_to_import())
+        self.import_clicked.emit(self.get_keys(include=True), self.get_loops(include=True))
 
     def show_import_window(self) -> None:
         row = 0
@@ -64,8 +64,8 @@ def show_import_window(self) -> None:
         self._set_label()
 
     def _save_selection(self):
-        self.settings.save_key_value('do_not_import_keys', self._get_keys_to_exclude())
-        self.settings.save_key_value('do_not_import_loops', self._get_loops_to_exclude())
+        self.settings.save_key_value('do_not_import_keys', self.get_keys(include=False))
+        self.settings.save_key_value('do_not_import_loops', self.get_loops(include=False))
 
     def _empty_saved_selection(self):
         self.settings.save_key_value('do_not_import_keys', [])
@@ -85,7 +85,7 @@ def _get_excluded_items(self) -> Tuple[List[str], List[List[str]]]:
     def _set_label(self) -> None:
         self.ui.importInfoLabel.setText(f"The CIF to import contains {self.keys_to_import} keys "
                                         f"and {self.loops_to_import} loops from which "
-                                        f"{len(self.get_keys_to_import()) + len(self.get_loops_to_import())} "
+                                        f"{len(self.get_keys(include=True)) + len(self.get_loops(include=True))} "
                                         f"are selected for import.")
 
     def _add_checkbox(self, text: str, row: int, col: QtWidgets.QTableWidget, checked: bool = False):
@@ -97,44 +97,25 @@ def _add_checkbox(self, text: str, row: int, col: QtWidgets.QTableWidget, checke
         col.setCellWidget(row, 0, checkbox)
         checkbox.setChecked(checked)
 
-    def get_keys_to_import(self) -> List[str]:
+    def get_keys(self, include: bool) -> List[str]:
         keys = []
         rows = self.ui.importTable_keys.rowCount()
         for row in range(rows):
             widget: QtWidgets.QCheckBox = self.ui.importTable_keys.cellWidget(row, 0)
-            if widget and widget.isChecked():
+            if widget and widget.isChecked() == include:
                 keys.append(widget.text())
         return keys
 
-    def _get_keys_to_exclude(self) -> List[str]:
-        keys = []
-        rows = self.ui.importTable_keys.rowCount()
-        for row in range(rows):
-            widget: QtWidgets.QCheckBox = self.ui.importTable_keys.cellWidget(row, 0)
-            if widget and not widget.isChecked():
-                keys.append(widget.text())
-        return keys
-
-    def get_loops_to_import(self) -> List[List[str]]:
+    def get_loops(self, include: bool) -> List[List[str]]:
         loops = []
         rows = self.ui.importTable_loops.rowCount()
         for row in range(rows):
             widget: QtWidgets.QCheckBox = self.ui.importTable_loops.cellWidget(row, 0)
-            if widget and widget.isChecked():
+            if widget and widget.isChecked() == include:
                 loop: List[str] = widget.text().splitlines(keepends=False)
                 loops.append(loop)
         return loops
 
-    def _get_loops_to_exclude(self) -> List[str]:
-        loops = []
-        rows = self.ui.importTable_loops.rowCount()
-        for row in range(rows):
-            widget: QtWidgets.QCheckBox = self.ui.importTable_loops.cellWidget(row, 0)
-            if widget and not widget.isChecked():
-                loop: List[str] = widget.text().splitlines(keepends=False)
-                loops.append(loop[0])
-        return loops
-
     def do_not_import_this_key(self, key: str) -> bool:
         value = self.import_cif[key]
         if value == '?' or value.strip() == '':

tests/test_import_cif.py

@@ -15,6 +15,7 @@ def setUp(self) -> None:
         targetcif = CifContainer('test-data/p21c-copy.cif')
         settings = FinalCifSettings()
         self.imp = ImportSelector(None, import_cif=imp_cif, target_cif=targetcif, settings=settings)
+        self.imp._empty_saved_selection()
         self.imp.show_import_window()
 
     def tearDown(self) -> None:
@@ -25,10 +26,14 @@ def test_keys_to_import(self):
         self.assertEqual(1, self.imp.loops_to_import)
 
     def test_import_methods(self):
-        self.assertEqual(['_foo_bar', '_hello'], self.imp.get_keys_to_import())
+        self.assertEqual(['_foo_bar', '_hello'], self.imp.get_keys(include=True))
         self.assertEqual([['_my_atom_type_symbol',
                            '_my_atom_type_description',
-                           '_my_atom_type_scat_dispersion_real']], self.imp.get_loops_to_import())
+                           '_my_atom_type_scat_dispersion_real']], self.imp.get_loops(include=True))
+
+    def test_import_methods_excluded(self):
+        self.assertEqual(['_cell_length_a'], self.imp.get_keys(include=False))
+        self.assertEqual([], self.imp.get_loops(include=False))
 
     def test_other(self):
         self.assertEqual('import_cif.cif', self.imp.import_cif.filename)