Support for ase.Atoms objects was deprecated for a while in spglib.standardize_cell and finally removed in the v2.3 release. However, these inputs are now more or less silently ignored and None values are returned instead of raising an error. This can lead to very surprising behavior for users. For example:
In [1]: from ase.build import bulk
In [2]: from spglib import standardize_cell
In [3]: atoms = bulk('Si')
In [4]: standardize_cell(atoms)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
Cell In[4], line 1
----> 1 standardize_cell(atoms)
File ~/.mambaforge/envs/aiida-py311/lib/python3.11/site-packages/spglib/spglib.py:1305, in standardize_cell(cell, to_primitive, no_idealize, symprec, angle_tolerance)
1276 """Return standardized cell. When the search failed, ``None`` is returned.
1277
1278 Parameters
(...)
1301 shown in the spglib (C-API) document.
1302 """
1303 _set_no_error()
-> 1305 lattice, _positions, _numbers, _ = _expand_cell(cell)
1306 if lattice is None:
1307 return None
File ~/.mambaforge/envs/aiida-py311/lib/python3.11/site-packages/spglib/spglib.py:1985, in _expand_cell(cell)
1984 def _expand_cell(cell):
-> 1985 lattice = np.array(np.transpose(cell[0]), dtype="double", order="C")
1986 positions = np.array(cell[1], dtype="double", order="C")
1987 numbers = np.array(cell[2], dtype="intc")
TypeError: float() argument must be a string or a real number, not 'Atom'But it can be even worse, with the function returning None:
In [5]: standardize_cell(atoms.cell)
The standardize_cell function expects as the first argument a tuple called cell which contains the lattice definition, the atomic positison and their numbers. This is not very intuitive, but at the very least I think the function should raise if it gets anything that doesn't match the expected signature.
For example, the function could have at the beginning something like
def standardize_cell(cell):
if not isinstance(cell, tuple) and not len(cell) in (3, 4):
raise ValueError(f'`cell` should be a tuple of length 3 or 4 but got: {cell})Since the structure of cell is not trivial, ideally it would contain a link to the docs that explain it.
Support for
ase.Atomsobjects was deprecated for a while inspglib.standardize_celland finally removed in thev2.3release. However, these inputs are now more or less silently ignored andNonevalues are returned instead of raising an error. This can lead to very surprising behavior for users. For example:But it can be even worse, with the function returning
None:The
standardize_cellfunction expects as the first argument a tuple calledcellwhich contains the lattice definition, the atomic positison and their numbers. This is not very intuitive, but at the very least I think the function should raise if it gets anything that doesn't match the expected signature.For example, the function could have at the beginning something like
Since the structure of
cellis not trivial, ideally it would contain a link to the docs that explain it.