OpenBabelReader and Tests

[lunamorrow]

In reference to #5. This is a draft of the code with the initial functionality starting to be roughly scaffolded out for feedback.

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[pep8speaks]

Hello @lunamorrow! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file mda_openbabel_converter/OpenBabel.py:

Line 52:13: E128 continuation line under-indented for visual indent
Line 53:13: E128 continuation line under-indented for visual indent
Line 59:80: E501 line too long (81 > 79 characters)

• In the file mda_openbabel_converter/data/files.py:

Line 17:61: W292 no newline at end of file

• In the file mda_openbabel_converter/tests/test_openbabel_reader.py:

Line 19:1: E302 expected 2 blank lines, found 1
Line 35:13: E128 continuation line under-indented for visual indent
Line 36:13: E128 continuation line under-indented for visual indent

Comment last updated at 2024-08-06 01:37:45 UTC

[exs-cbouy]

Could you also add a test for mols without any coordinates making sure that they are all nan?

[exs-cbouy]

Nice work Luna!

[xhgchen]

Great work! Loving the progress :)

[lunamorrow]

Could you also add a test for mols without any coordinates making sure that they are all nan?

@exs-cbouy It looks like null position in OpenBabel is stored as a (0, 0, 0) vector. I don't want to report that as a position when its not, so I'm thinking that I can check that if the whole molecules position vectors are (0, 0, 0) that there is no position info and to set it to nan (or not even set it) and issue a warning. That will prevent molecules with an atom at a valid (0, 0, 0) position from being flagged. Thoughts?

Edit: I've got it, just had to change my conditions for empty coordinates. Pushing changes now.

[exs-cbouy]

Could potentially be as simple as (haven't tried it though):

Suggested change

	coordinates = np.array([
	[(coords := atom.GetVector()).GetX(),
	coords.GetY(),
	coords.GetZ()] for atom in OBMolAtomIter(filename)],
	dtype=np.float32)
	coordinates = np.array(filename.GetConformers(), dtype=np.float32)

Also we overlooked that the current implementation is missing one dimension for the number of conformers

[hmacdope]

LGTM