Repository: biotite-dev/biotite (python)

Issue:
altloc_id missing from get_structure() output
The last few recent commits have altered the behavior of get_structure() so that its output is missing the altloc_id annotation. I can't tell if this is expected behavior or a bug, however the docstring implies that its a bug.
```
    altloc : {'first', 'occupancy', 'all'}
        This parameter defines how *altloc* IDs are handled:
            - ``'first'`` - Use atoms that have the first *altloc* ID
              appearing in a residue.
            - ``'occupancy'`` - Use atoms that have the *altloc* ID
              with the highest occupancy for a residue.
            - ``'all'`` - Use all atoms.
              Note that this leads to duplicate atoms.
              When this option is chosen, the ``altloc_id`` annotation
              array is added to the returned structure.
```

So it seems that either its a bug or the docstring should get updated.

Minimal example to reproduce:

```
from biotite.database import rcsb
import biotite.structure.io.pdbx as pdbx

ciffile = rcsb.fetch("4zxb", "cif")
cif = pdbx.CIFFile.read(ciffile)
array = pdbx.get_structure(
    cif,
    model=1,
    altloc="all",
    use_author_fields=False,
)
array.get_annotation_categories()
```

current output from the latest commit is
```
['chain_id',
 'res_id',
 'ins_code',
 'res_name',
 'hetero',
 'atom_name',
 'element']
```
rolling back to `d0ae83d` gives
```
['chain_id',
 'res_id',
 'ins_code',
 'res_name',
 'hetero',
 'atom_name',
 'element',
 'altloc_id']
```

Pull request:
Fix #794
Fixes #794 

Patch:
diff --git a/pyproject.toml b/pyproject.toml
index 4e857dfc..c0eeda8e 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -4,12 +4,12 @@ requires-python = ">=3.11"
 description = "A comprehensive library for computational molecular biology"
 readme = "README.rst"
 authors = [{name = "The Biotite contributors"}]
-license = "BSD-3-Clause"
-license-files = ["LICENSE.rst"]
+license = {"file" = "LICENSE.rst"}
 classifiers = [
     "Development Status :: 5 - Production/Stable",
     "Intended Audience :: Developers",
     "Intended Audience :: Science/Research",
+    "License :: OSI Approved :: BSD License",
     "Natural Language :: English",
     "Operating System :: POSIX :: Linux",
     "Operating System :: MacOS",
diff --git a/src/biotite/structure/io/pdbx/convert.py b/src/biotite/structure/io/pdbx/convert.py
index 94b3a15c..35520bd3 100644
--- a/src/biotite/structure/io/pdbx/convert.py
+++ b/src/biotite/structure/io/pdbx/convert.py
@@ -766,8 +766,10 @@ def _filter_altloc(array, atom_site, altloc):
     altloc_ids = atom_site.get("label_alt_id")
     occupancy = atom_site.get("occupancy")
 
-    # Filter altloc IDs and return
-    if altloc_ids is None or (altloc_ids.mask.array != MaskValue.PRESENT).all():
+    if altloc == "all":
+        array.set_annotation("altloc_id", altloc_ids.as_array(str))
+        return array, atom_site
+    elif altloc_ids is None or (altloc_ids.mask.array != MaskValue.PRESENT).all():
         # No altlocs in atom_site category
         return array, atom_site
     elif altloc == "occupancy" and occupancy is not None:
@@ -779,9 +781,6 @@ def _filter_altloc(array, atom_site, altloc):
     elif altloc == "first":
         mask = filter_first_altloc(array, altloc_ids.as_array(str))
         return array[..., mask], _filter(atom_site, mask)
-    elif altloc == "all":
-        array.set_annotation("altloc_id", altloc_ids.as_array(str))
-        return array, atom_site
     else:
         raise ValueError(f"'{altloc}' is not a valid 'altloc' option")
 


Test patch:
diff --git a/.github/workflows/test_and_deploy.yml b/.github/workflows/test_and_deploy.yml
index f830691b..73633f87 100644
--- a/.github/workflows/test_and_deploy.yml
+++ b/.github/workflows/test_and_deploy.yml
@@ -106,7 +106,7 @@ jobs:
       - uses: actions/checkout@v4
       - name: Install cibuildwheel
         # MAKE SURE THIS STAYS IN SYNC WITH THE LOWER GHA cibuildwheel
-        run: pipx install cibuildwheel==2.23.3
+        run: pipx install cibuildwheel==2.20.0
       - id: set-matrix
         run: |
           MATRIX=$(
@@ -157,7 +157,7 @@ jobs:
 
       - name: Build & (optionally) test wheels
         # MAKE SURE THIS STAYS IN SYNC WITH THE UPPER pipx call to cibuildwheel
-        uses: pypa/cibuildwheel@v2.23.3
+        uses: pypa/cibuildwheel@v2.20.0
         with:
           only: ${{ matrix.dist }}
 
diff --git a/tests/structure/io/test_pdbx.py b/tests/structure/io/test_pdbx.py
index 97e3686b..e12047cf 100644
--- a/tests/structure/io/test_pdbx.py
+++ b/tests/structure/io/test_pdbx.py
@@ -148,6 +148,35 @@ def test_conversion(monkeypatch, tmpdir, format, path, model, find_matches_by_di
     assert test_atoms.coord.tolist() == ref_atoms.coord.tolist()
 
 
+@pytest.mark.parametrize(
+    "pdb_id",
+    [
+        "1aki",  # has no altloc IDs
+        "3o5r",  # has altloc IDs
+    ],
+)
+def test_filter_altloc(pdb_id):
+    """
+    Check if the different ``altloc`` options give the expected results.
+    """
+    pdbx_file = pdbx.BinaryCIFFile.read(join(data_dir("structure"), f"{pdb_id}.bcif"))
+    atoms = {}
+    for altloc in ["first", "occupancy", "all"]:
+        atoms[altloc] = pdbx.get_structure(pdbx_file, model=1, altloc=altloc)
+
+    # The 'altloc_id' annotation should only be present in the 'altloc=all' case
+    assert "altloc_id" in atoms["all"].get_annotation_categories()
+    assert "altloc_id" not in atoms["first"].get_annotation_categories()
+    assert "altloc_id" not in atoms["occupancy"].get_annotation_categories()
+    # Independent of which altloc atom is selected, only one atom must be selected...
+    assert atoms["occupancy"].array_length() == atoms["first"].array_length()
+    # ...with the exception of the 'altloc=all' case in which more atoms are selected
+    if np.any(atoms["all"].altloc_id != "."):
+        assert atoms["all"].array_length() > atoms["first"].array_length()
+    else:
+        assert atoms["all"].array_length() == atoms["first"].array_length()
+
+
 @pytest.mark.parametrize(
     "format, path",
     itertools.product(