add cif::Table::ensure_loop()

it replaces convert_pair_to_loop(), fixing a bug (positions were not updated)

ensure_loop() has docs and python bindings, and is no longer called
implicitly from append_row().

docs/cif.rst

@@ -1133,6 +1133,15 @@ If Table's data is in Loop, the Loop class can be accessed using::
   >>> table.loop     # None for tag-value pairs
   <gemmi.cif.Loop 18 x 4>
 
+Tag-value pairs can be converted to a loop using:
+
+.. doctest::
+
+  >>> table.ensure_loop()
+
+but make sure to call it only when the table represents the whole category
+(see example :ref:`below <append_row>`).
+
 If a prefix was specified when calling find, the prefix length is stored
 and the prefix can be retrieved::
 
@@ -1250,19 +1259,44 @@ and a property::
   >>> row.row_index
   9
 
+Individual values in a row can be directly modified.
+This includes tags, which are accessible as a special row.
+As an example, let us swap two names
+(these two tend to have identical values, so no one will notice):
+
+.. literalinclude:: code/cif_cc.cpp
+   :language: cpp
+   :lines: 32-34
+
+.. doctest::
+
+  >>> tags = block.find('_atom_site.', ['label_atom_id', 'auth_atom_id']).tags
+  >>> tags[0], tags[1] = tags[1], tags[0]
+
 ----
 
-We can append a row to Table (function `Table::append_row`):
+.. _append_row:
+
+Function `Table::append_row` appends a row.
+It won't work if the table is composed of tag-value pairs (which isn't
+the case here, but let's make this example as general as possible),
+so we call `Table::ensure_loop()` first.
+This function, in turn, won't work well if the table contains only a subset
+of the category, thus `find_mmcif_category()`:
 
 .. doctest::
 
+  >>> block.find_mmcif_category('_entity_poly_seq.').ensure_loop()
   >>> table.append_row(['3', '4', 'new'])
   >>> table[-1]
   <gemmi.cif.Table.Row: 3 4 new>
   >>> _.row_index
   18
 
-move a row to a different position:
+If new tags are added to this category in the future,
+the corresponding values in appended rows will be filled with `.`.
+
+We can also move a row to a different position:
 
 .. doctest::
 
@@ -1283,9 +1317,9 @@ C++ function `Table::remove_rows` or Python `__delitem__` with slice:
 
   >>> del table[12:15]
 
-As is usual with any containers (in both Python and C++)
+As is usual with any containers (in both Python and C++),
 if you want to remove items while iterating over them,
-it's better to iterate backward.
+the iteration must be done backward.
 Here is an example that removes atoms with zero occupancy:
 
 .. doctest::
@@ -1298,19 +1332,6 @@ Here is an example that removes atoms with zero occupancy:
   ...
   >>> doc.write_file('out.cif')
 
-Individual tags and values can also be modified.
-As an example, let us swap two tag names
-(these two tend to have identical values, so no one will notice):
-
-.. literalinclude:: code/cif_cc.cpp
-   :language: cpp
-   :lines: 32-34
-
-.. doctest::
-
-  >>> tags = block.find('_atom_site.', ['label_atom_id', 'auth_atom_id']).tags
-  >>> tags[0], tags[1] = tags[1], tags[0]
-
 Column-wise access
 ------------------
 

include/gemmi/cifdoc.hpp

@@ -425,7 +425,8 @@ struct Table {
 
   void erase();
 
-  void convert_pair_to_loop();
+  /// if it's pairs, convert it to loop
+  void ensure_loop();
 
   // It is not a proper input iterator, but just enough for using range-for.
   struct iterator {
@@ -747,7 +748,7 @@ template <typename T> void Table::append_row(T new_values) {
   if (new_values.size() != width())
     fail("append_row(): wrong row length");
   if (!loop_item)
-    convert_pair_to_loop();
+    fail("append_row(): data is not in loop, call ensure_loop() first");
   Loop& loop = loop_item->loop;
   size_t cur_size = loop.values.size();
   loop.values.resize(cur_size + loop.width(), ".");
@@ -760,8 +761,7 @@ inline void Table::remove_rows(int start, int end) {
   if (!ok())
     // this function is used mostly through remove_row()
     fail("remove_row(): table not found");
-  if (!loop_item)
-    convert_pair_to_loop();
+  ensure_loop();  // should we fail instead if we have pairs?
   Loop& loop = loop_item->loop;
   size_t start_pos = start * loop.width();
   size_t end_pos = end * loop.width();
@@ -789,18 +789,20 @@ inline void Table::erase() {
   positions.clear();
 }
 
-inline void Table::convert_pair_to_loop() {
-  assert(loop_item == nullptr);
+inline void Table::ensure_loop() {
+  if (loop_item)
+    return;
   Item new_item(LoopArg{});
   new_item.loop.tags.resize(positions.size());
   new_item.loop.values.resize(positions.size());
+  loop_item = &bloc.items.at(positions[0]);
   for (size_t i = 0; i != positions.size(); ++i) {
     Item& item = bloc.items[positions[i]];
     new_item.loop.tags[i].swap(item.pair[0]);
     new_item.loop.values[i].swap(item.pair[1]);
     item.erase();
+    positions[i] = i;
   }
-  loop_item = &bloc.items.at(positions[0]);
   loop_item->set_value(std::move(new_item));
 }
 

include/gemmi/to_chemcomp.hpp

@@ -19,8 +19,7 @@ inline void add_chemcomp_to_block(const ChemComp& cc, cif::Block& block) {
       for (char c = 'x'; c <= 'z'; ++c)
         tags.emplace_back(1, c);
     cif::Table tab = block.find_or_add("_chem_comp_atom.", tags);
-    if (!tab.loop_item)
-      tab.convert_pair_to_loop();
+    tab.ensure_loop();
     size_t pos = tab.length();
     cif::Loop& loop = tab.loop_item->loop;
     loop.values.resize(loop.values.size() + loop.width() * cc.atoms.size(), ".");

python/cif.cpp

@@ -399,6 +399,7 @@ void add_cif(py::module& cif) {
     .def("find_column", &Table::find_column, py::arg("tag"),
          py::keep_alive<0, 1>())
     .def("erase", &Table::erase)
+    .def("ensure_loop", &Table::ensure_loop)
     .def("append_row", &Table::append_row<std::vector<std::string>>,
          py::arg("new_values"))
     .def("remove_row", &Table::remove_row, py::arg("row_index"))