search

CCP-NC / magresview-2

MagresView 2.0 - NMR crystallography visualisation app
https://ccp-nc.github.io/magresview-2/
MIT License
0 stars 2 forks source link

CIF read generates error #12

Closed dch0ph closed 1 year ago

dch0ph commented 1 year ago

CONGRS_geomopt-out.zip

The attached CIF (zipped to allow GutHib to accept) fails to load with the error:

image

OK, reading CIFs is not a top priority for MagresView, but it does suggest some fragility in the underlying crystal viewer.

MV1 reads the file, but makes a mess of "loading as molecule".

jkshenton commented 1 year ago

Thanks @dch0ph.

It seems to be a problem with the library used to parse CIF files: https://github.com/CCP-NC/crystcif-parse rather than MV2 or crystvis-js.

Your attached CIF file doesn't contain _atom_site_type_symbol tags (i.e. the element symbol), only the labels. Adding these in manually makes everything work nicely (including the 'load as molecule') in MV2.

I'm guessing that other parsers/visualisers fall back on extracting the element from the labels. We could use regex to split the labels at the first non-alpha character and assume the first bit is the element symbol.

dch0ph commented 1 year ago

Good spot!

MV2/crystvis-js could perhaps handle this better, but as you say the main problem is the CIF parsing:

From the official documentation:

image

crystcif-parse is in the wrong, since the CIF is valid. The documentation also implies ("component 0") that the _atom_site_type_symbol must be extractable from the _atom_site_label in this case.

CIF is a horridly complicated format, and I do worry about the wisdom / maintainability of both a CIF parser and crystal viewer...