Repository: biotite-dev/biotite (python)

Issue:
Failed to deserialize category 'entity' with ValueError: No closing quotation
`1n5m.cif` pdbx_description in entity category has no closing quotation:
`'2,2',2"-[1,2,3-BENZENE-TRIYLTRIS(OXY)]TRIS[N,N,N-TRIETHYLETHANAMINIUM]'`

There are a total of 3 problematic examples in the wwPDB database: 1n5m, 6szp, 1tsl

I can fix it, but I'm not sure about the best way to do so. Do you have any suggestions?


code and trackback

```python
# test_example.py
# biotite=0.40.0
import biotite.structure.io.pdbx as pdbx
from biotite.database import rcsb
import biotite.structure as struc
import biotite.structure.io as strucio

# There are a total of 3 problematic examples in the wwPDB database.
pdb_id = "1n5m"
# pdb_id = "6szp"
# pdb_id = "1tsl"

try:
    cif_file = pdbx.CIFFile.read(f"/tmp/{pdb_id}.cif")
except:
    rcsb.fetch(pdb_id, "cif", target_path="/tmp")
    cif_file = pdbx.CIFFile.read(f"/tmp/{pdb_id}.cif")

entity = cif_file.block["entity"]
```

```console
$ python test_example.py 
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/biotite/structure/io/pdbx/cif.py", line 658, in __getitem__
    category = CIFCategory.deserialize(category, expect_whitespace)
  File "/usr/local/lib/python3.9/dist-packages/biotite/structure/io/pdbx/cif.py", line 382, in deserialize
    category_dict = CIFCategory._deserialize_looped(
  File "/usr/local/lib/python3.9/dist-packages/biotite/structure/io/pdbx/cif.py", line 486, in _deserialize_looped
    values = shlex.split(data_line)
  File "/usr/lib/python3.9/shlex.py", line 315, in split
    return list(lex)
  File "/usr/lib/python3.9/shlex.py", line 300, in __next__
    token = self.get_token()
  File "/usr/lib/python3.9/shlex.py", line 109, in get_token
    raw = self.read_token()
  File "/usr/lib/python3.9/shlex.py", line 191, in read_token
    raise ValueError("No closing quotation")
ValueError: No closing quotation

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/test_example.py", line 18, in <module>
    entity = cif_file.block["entity"]
  File "/usr/local/lib/python3.9/dist-packages/biotite/structure/io/pdbx/cif.py", line 660, in __getitem__
    raise DeserializationError(
biotite.structure.io.pdbx.DeserializationError: Failed to deserialize category 'entity'


$ grep '^_entity.id' -A 20 /tmp/1n5m.cif 
_entity.id 
_entity.type 
_entity.src_method 
_entity.pdbx_description 
_entity.formula_weight 
_entity.pdbx_number_of_molecules 
_entity.pdbx_ec 
_entity.pdbx_mutation 
_entity.pdbx_fragment 
_entity.details 
1 polymer     man acetylcholinesterase                                                     59592.309 2   3.1.1.7 ? 
'CATALYTIC DOMAIN' ? 
2 branched    man 'alpha-L-fucopyranose-(1-6)-2-acetamido-2-deoxy-beta-D-glucopyranose'    367.349   1   ?       ? ? ? 
3 non-polymer syn 'IODIDE ION'                                                             126.904   10  ?       ? ? ? 
4 non-polymer syn 'HEXAETHYLENE GLYCOL'                                                    282.331   1   ?       ? ? ? 
5 non-polymer syn '2,2',2"-[1,2,3-BENZENE-TRIYLTRIS(OXY)]TRIS[N,N,N-TRIETHYLETHANAMINIUM]' 510.816   1   ?       ? ? ? 
6 non-polymer man 2-acetamido-2-deoxy-beta-D-glucopyranose                                 221.208   1   ?       ? ? ? 
7 non-polymer syn 'CARBONATE ION'                                                          60.009    1   ?       ? ? ? 
8 non-polymer syn 'TETRAETHYLENE GLYCOL'                                                   194.226   1   ?       ? ? ? 
9 water       nat water                                                                    18.015    551 ?       ? ? ? 
#                 1 
```

Pull request description:
Handle embedded quote in mmcif
 fix #570

use 3 regex patterns to match fields in one line for handle embed quote in mmcif file:
```
  single_quote_pattern = r"('(?:'(?! )|[^'])*')(?:\s|$)"
  double_quote_pattern = r'("(?:"(?! )|[^"])*")(?:\s|$)'
  unquoted_pattern = r"([^\s]+)"
```

GPT4 explain single_quote_pattern:

> This regex `single_quote_pattern = r"('(?:'(?! )|[^'])*')(?:\s|$)"` is engineered to identify and extract substrings enclosed in single quotes from a larger text, with a particular sensitivity to handle internal apostrophes correctly. Let's dissect this expression to understand how it functions:
> 
> 1. **`'`**: This matches the opening single quote `'` of the target substring.
> 
> 2. **`(?: ... )`**: This is a non-capturing group, which means it groups the contained pattern parts without storing the matched substring. This is used here mainly for grouping purposes without needing backreferences.
> 
> 3. **`'(?! )`**: This is a negative lookahead assertion that matches a single quote `'` only if it's not immediately followed by a space ` `. This allows the regex to match apostrophes within words (like in contractions such as `don't`) without treating them as the end of the quoted substring.
> 
> 4. **`|`**: The logical OR operator presents an alternative within the non-capturing group. It separates the negative lookahead for internal apostrophes from the next part of the pattern.
> 
> 5. **`[^']`**: This is a negated character class that matches any character except a single quote `'`. This part of the expression ensures that the regex consumes all characters within the quotes until it encounters the next single quote, which might signify the end of the quoted substring.
> 
> 6. **`*`**: This quantifier applies to the non-capturing group, allowing the contained pattern to repeat any number of times — including zero times — thus enabling the regex to match quoted substrings of any length.
> 
> 7. **`'`**: Matches the closing single quote of the substring.
> 
> 8. **`(?:\s|$)`**: Another non-capturing group that operates as a condition for what follows the closing quote. It matches either:
>    - **`\s`**: A whitespace character, ensuring that the quoted substring is followed by a space, or
>    - **`$`**: The end of a line or string, allowing for the quoted substring to appear at the end of the text.
> 
> #### The Key Points:
> - The pattern is designed to efficiently target substrings enclosed in single quotes within a larger string or document.
> - It smartly handles situations where an apostrophe is part of the enclosed text (like in contractions) without mistakenly recognizing it as the end of the quoted section.
> - By requiring the quoted substring to be followed by a space or the end of the text, it imposes a sensible boundary condition to identify discrete quoted substrings within a flow of text.
> 
> This regex could be particularly useful in text parsing applications where accurately distinguishing between quoted strings and regular text is crucial, such as in natural language processing tasks, data extraction, or in developing syntax highlighters for code editors.


Patch:
diff --git a/doc/switcher.py b/doc/switcher.py
index 97471561..b34cad86 100644
--- a/doc/switcher.py
+++ b/doc/switcher.py
@@ -69,6 +69,9 @@ def create_switcher_json(file_path, min_tag, n_versions):
     """
     version_config = []
     for version in _get_previous_versions(min_tag, n_versions)[::-1]:
+        if version.patch != 0:
+            # Documentation is not uploaded for patch versions
+            continue
         version_config.append({
             "name": f"{version.major}.{version.minor}",
             "version": str(version),
diff --git a/src/biotite/structure/bonds.pyx b/src/biotite/structure/bonds.pyx
index e1b28371..8dc6e3b0 100644
--- a/src/biotite/structure/bonds.pyx
+++ b/src/biotite/structure/bonds.pyx
@@ -1005,7 +1005,7 @@ class BondList(Copyable):
         # maximum and redundant bond calculation
         merged_bond_list._bonds = merged_bonds
         merged_bond_list._max_bonds_per_atom = max(
-            self._max_bonds_per_atom, merged_bond_list._max_bonds_per_atom
+            self._max_bonds_per_atom, bond_list._max_bonds_per_atom
         )
         return merged_bond_list
 
diff --git a/src/biotite/structure/io/pdbx/cif.py b/src/biotite/structure/io/pdbx/cif.py
index 6ca9bfe6..b7f8780a 100644
--- a/src/biotite/structure/io/pdbx/cif.py
+++ b/src/biotite/structure/io/pdbx/cif.py
@@ -6,8 +6,8 @@ __name__ = "biotite.structure.io.pdbx"
 __author__ = "Patrick Kunzmann"
 __all__ = ["CIFFile", "CIFBlock", "CIFCategory", "CIFColumn", "CIFData"]
 
+import re
 import itertools
-import shlex
 from collections.abc import MutableMapping, Sequence
 import numpy as np
 from .component import _Component, MaskValue
@@ -449,7 +449,7 @@ class CIFCategory(_Component, MutableMapping):
         """
         category_dict = {}
         for line in lines:
-            parts = shlex.split(line)
+            parts = _split_one_line(line)
             column_name = parts[0].split(".")[1]
             column = parts[1]
             category_dict[column_name] = CIFColumn(column)
@@ -480,12 +480,11 @@ class CIFCategory(_Component, MutableMapping):
         column_names = itertools.cycle(column_names)
         for data_line in data_lines:
             # If whitespace is expected in quote protected values,
-            # use standard shlex split
+            # use regex-based _split_one_line() to split
             # Otherwise use much more faster whitespace split
-            # and quote removal if applicable,
-            # bypassing the slow shlex module
+            # and quote removal if applicable.
             if expect_whitespace:
-                values = shlex.split(data_line)
+                values = _split_one_line(data_line)
             else:
                 values = data_line.split()
                 for k in range(len(values)):
@@ -652,7 +651,7 @@ class CIFBlock(_Component, MutableMapping):
                 # Special optimization for "atom_site":
                 # Even if the values are quote protected,
                 # no whitespace is expected in escaped values
-                # Therefore slow shlex.split() call is not necessary
+                # Therefore slow regex-based _split_one_line() call is not necessary
                 if key == "atom_site":
                     expect_whitespace = False
                 else:
@@ -973,11 +972,11 @@ def _to_single(lines, is_looped):
                 j += 1
             if is_looped:
                 # Create a line for the multiline string only
-                processed_lines[out_i] = shlex.quote(multi_line_str)
+                processed_lines[out_i] = f"'{multi_line_str}'"
                 out_i += 1
             else:
                 # Append multiline string to previous line
-                processed_lines[out_i - 1] += " " + shlex.quote(multi_line_str)
+                processed_lines[out_i - 1] += " " + f"'{multi_line_str}'"
             in_i = j + 1
 
         elif not is_looped and lines[in_i][0] != "_":
@@ -1024,6 +1023,34 @@ def _multiline(value):
     return value
 
 
+def _split_one_line(line):
+    """
+    Split a line into its fields.
+    Supporting embedded quotes (' or "), like `'a dog's life'` to  `a dog's life`
+    """
+    # Define the patterns for different types of fields
+    single_quote_pattern = r"('(?:'(?! )|[^'])*')(?:\s|$)"
+    double_quote_pattern = r'("(?:"(?! )|[^"])*")(?:\s|$)'
+    unquoted_pattern = r"([^\s]+)"
+
+    # Combine the patterns using alternation
+    combined_pattern = (
+        f"{single_quote_pattern}|{double_quote_pattern}|{unquoted_pattern}"
+    )
+
+    # Find all matches
+    matches = re.findall(combined_pattern, line)
+
+    # Extract non-empty groups from the matches
+    fields = []
+    for match in matches:
+        field = next(group for group in match if group)
+        if field[0] == field[-1] == "'" or field[0] == field[-1] == '"':
+            field = field[1:-1]
+        fields.append(field)
+    return fields
+
+
 def _arrayfy(data):
     if not isinstance(data, (Sequence, np.ndarray)) or isinstance(data, str):
         data = [data]


Test patch:
diff --git a/tests/structure/test_bonds.py b/tests/structure/test_bonds.py
index 7d8c19d7..a5474ffd 100644
--- a/tests/structure/test_bonds.py
+++ b/tests/structure/test_bonds.py
@@ -133,6 +133,19 @@ def test_modification(bond_list):
                                              [1, 4, 0]]
 
 
+def test_add_two_bond_list():
+    """
+    Test adding two `BondList` objects.
+    """
+    bond_list1 = struc.BondList(2, np.array([(0,1)])) # max_bond_per_atom=1
+    bond_list2 = struc.BondList(3, np.array([(0,1),(0,2)])) # max_bond_per_atom=2
+    added_list = bond_list1 + bond_list2
+    assert added_list._max_bonds_per_atom == 2
+    assert added_list.get_bonds(2)[0].tolist() == [3, 4]
+    assert added_list.as_array().tolist() == [[0, 1, 0],
+                                              [2, 3, 0],
+                                              [2, 4, 0]]
+
 def test_contains(bond_list):
     """
     Test whether `BondList` correctly identifies whether it contains a
diff --git a/tests/structure/test_pdbx.py b/tests/structure/test_pdbx.py
index 68d16627..ec32bbd3 100644
--- a/tests/structure/test_pdbx.py
+++ b/tests/structure/test_pdbx.py
@@ -59,6 +59,22 @@ def test_escape(string, looped):
     assert test_value == ref_value
 
 
+@pytest.mark.parametrize(
+    "cif_line, expected_fields",
+    [
+        ["'' 'embed'quote' ", ['', "embed'quote"]],
+        ['2 "embed"quote" "\t\n"', ['2', 'embed"quote', '\t\n']],
+        [" 3 '' \"\" 'spac e' 'embed\"quote'", ['3', '', '', 'spac e', 'embed"quote']],
+        ["''' \"\"\" ''quoted''", ["'", '"', "'quoted'"]]
+    ]
+)
+def test_split_one_line(cif_line, expected_fields):
+    """
+    Test whether values that have an embedded quote are properly escaped.
+    """
+    assert pdbx.cif._split_one_line(cif_line) == expected_fields
+
+
 @pytest.mark.parametrize(
     "format, path, model",
     itertools.product(