danieleongari / CURATED-COFs

Clean, Uniform and Refined with Automatic Tracking from Experimental Database (CURATED) COFs
MIT License
34 stars 11 forks source link

use of outdated CIF keys, breaking the parser of ASE 3.20 #16

Closed ltalirz closed 3 years ago

ltalirz commented 4 years ago

P.S. For some reason, from version 3.20, ASE starts complaining about the symmetry specification in current CIF files (now limiting ase version to <3.20) We should check whether updating the cif keys fixes this.

ltalirz commented 3 years ago

@danieleongari This is a very simple change that is necessary to comply with the latest CIF standard. Can we make this change or is there a parser "upstream" that does not understand these keys?

danieleongari commented 3 years ago

I'll change it in https://github.com/danieleongari/CURATED-COFs/pull/22, and we'll see if problems emerge

danieleongari commented 3 years ago

It loads fine with ASE-gui and Zeopp

ltalirz commented 3 years ago

sorry, which version of ase did you test?

I'm testing with ase 3.21.1 and on your updated branch I'm getting

In [5]: atoms = read('19456N2.cif')
----------------------------------------------------
AssertionError     Traceback (most recent call last)
<ipython-input-5-9db16add7238> in <module>
----> 1 atoms = read('19456N2.cif')

~/Applications/miniconda3/lib/python3.6/site-packages/ase/io/formats.py in read(filename, index, format, parallel, do_not_split_by_at_sign, **kwargs)
    681     else:
    682         return next(_iread(filename, slice(index, None), format, io,
--> 683                            parallel=parallel, **kwargs))
    684
    685

~/Applications/miniconda3/lib/python3.6/site-packages/ase/parallel.py in new_generator(*args, **kwargs)
    271             not kwargs.pop('parallel', True)):
    272             # Disable:
--> 273             for result in generator(*args, **kwargs):
    274                 yield result
    275             return

~/Applications/miniconda3/lib/python3.6/site-packages/ase/io/formats.py in _iread(filename, index, format, io, parallel, full_output, **kwargs)
    747     # Make sure fd is closed in case loop doesn't finish:
    748     try:
--> 749         for dct in io.read(fd, *args, **kwargs):
    750             if not isinstance(dct, dict):
    751                 dct = {'atoms': dct}

~/Applications/miniconda3/lib/python3.6/site-packages/ase/io/cif.py in read_cif(fileobj, index, store_tags, primitive_cell, subtrans_included, fractional_occupancies, reader)
    600     # Find all CIF blocks with valid crystal data
    601     images = []
--> 602     for block in parse_cif(fileobj, reader):
    603         if not block.has_structure():
    604             continue

~/Applications/miniconda3/lib/python3.6/site-packages/ase/io/cif.py in parse_cif_ase(fileobj)
    536             continue
    537
--> 538         yield parse_block(lines, line)
    539
    540

~/Applications/miniconda3/lib/python3.6/site-packages/ase/io/cif.py in parse_block(lines, line)
    497
    498 def parse_block(lines: List[str], line: str) -> CIFBlock:
--> 499     assert line.lower().startswith('data_')
    500     blockname = line.split('_', 1)[1].rstrip()
    501     tags = parse_items(lines, line)

AssertionError:

I've looked into it and it turns out the issue is a line with a space after the symmetry specification

E.g. In the case of FAPTOH.cif it fails with this

loop_
_symmetry_equiv_pos_as_xyz
 'x,y,z'

(last line has one space)

and it works with this

loop_
_symmetry_equiv_pos_as_xyz
 'x,y,z'

(last line is empty)

danieleongari commented 3 years ago

Right, I was testing most recent CIFs. Fixed (on Mac) with:

for f in *; do sed -i '' "s/^ *//" $f; done

See commit: