"Centre Structure" option removes Center of Geometry translations rather than Center of Mass

[rbdavid]

"Centre Structure" option removes Center of Geometry translations rather than Center of Mass

[BradyAJohnston]

I think giving the option in UI is a good idea.

You can use an enum to display the properties like so:

bpy.types.Scene.MN_center_type = bpy.props.EnumProperty(
    name="Method",
    items=(
        ('mass', "Mass", "Adjust the centre of mass to be at the world origin.", 0),
        ('centroid', "Centroid", "Adjust the centroid (ignoring mass) to be att he world origin.", 1)
    )
)

Then display that property on each of the panels for the import methods.

The panels are defined under io/wwpdb, io/local io/md for the relevant import methods.

[BradyAJohnston]

Yep please make a PR! Can have a look at it then :)

[BradyAJohnston]

I'm happy to include the masses in the data.py dictionaries - and would be something useful to have in MN going forward.

When importing via downloading from the PDB or parsing a local file, biotite is used and MDAnalysis is only used when importing via the MD import method. Trying to use methods from MDAnalysis while importing via biotite would get messy and I'd ideally want to avoid it.

If we get the masses from the data.py though, the com calculation could happen during import, or it could also be done via nodes (probably options for both).

If first you wanted to create a PR to include the masses that would be great. I can give some help with turning that into an attribute if you'd like. If you'd like to contribute a com calculation we can use that, otherwise that can be implemented later.

[BradyAJohnston]

Looking at that dictionary you linked as well - I have also been meaning to create a dictionary for converting from atomic_number to letters etc for potentially writing .pdb/.cif files. We could replace my existing dictionary with the ones you have there.

[rbdavid]

Before I make a PR, can you explain where the elements dictionary is used? I'm noticing instances of duplicate entries (ex: Na and NA both mapping to sodium), which suggest maybe the dictionary is used for interpreting atom names used in structure files. Similarly, there are some interesting entries such as D mapping to a carbon atom instead of deuterium. And a smattering of course-grained particle types also included in the dictionary.

Maybe a second dictionary is appropriate that contains a mapping between standard atom names used in structure files (originating from AMBER, CHARMM, OPLS force fields or crystallographic naming conventions that I'm not familiar with) and their associated element. This way, hopefully the ambiguity/diversity of atom name formatting gets accurately mapped to the element symbol and subsequently can be mapped to the element's atomic number/vdw radius/full name/ mass (info held within the elements dictionary).

Additionally, there are a whole bunch of 100 values in the 'vdw_radii' keys in the elements subdictionaries, which I suspect are just place-holders. Should a default vdw_radii value be set, for visualization purposes, that is overwritten if an atom has an element with a 'vdw_radii' key in its elements dictionary entry?

[BradyAJohnston]

Before I make a PR, can you explain where the elements dictionary is used?

They are used when creating the models inside of Blender, usually to translate from string to integer values, or for looking up values like VDW radii.

When parsing regular structure files:

MolecularNodes/molecularnodes/io/parse/molecule.py

Lines 384 to 389 in f3dbff0

	def att_atomic_number():
	atomic_number = np.array([
	data.elements.get(x, {'atomic_number': -1}).get("atomic_number")
	for x in np.char.title(array.element)
	])
	return atomic_number

MolecularNodes/molecularnodes/io/parse/molecule.py

Lines 438 to 445 in f3dbff0

	def att_vdw_radii():
	vdw_radii = np.array(list(map(
	# divide by 100 to convert from picometres to angstroms which is what all of coordinates are in
	lambda x: data.elements.get(
	x, {'vdw_radii': 100}).get('vdw_radii', 100) / 100,
	np.char.title(array.element)
	)))
	return vdw_radii * world_scale

When importing a MD trajectory:

MolecularNodes/molecularnodes/io/parse/mda.py

Lines 172 to 176 in f3dbff0

	def atomic_number(self) -> np.ndarray:
	return np.array(
	[data.elements.get(element,
	data.elements.get('X'))
	.get('atomic_number') for element in self.elements]

MolecularNodes/molecularnodes/io/parse/mda.py

Lines 180 to 185 in f3dbff0

	def vdw_radii(self) -> np.ndarray:
	# pm to Angstrom
	return np.array(
	[data.elements.get(element,
	data.elements.get('X'))
	.get('vdw_radii') for element in self.elements]) * 0.01 * self.world_scale

I'm noticing instances of duplicate entries (ex: Na and NA both mapping to sodium), which suggest maybe the dictionary is used for interpreting atom names used in structure files. Similarly, there are some interesting entries such as D mapping to a carbon atom instead of deuterium. And a smattering of course-grained particle types also included in the dictionary.
Maybe a second dictionary is appropriate that contains a mapping between standard atom names used in structure files (originating from AMBER, CHARMM, OPLS force fields or crystallographic naming conventions that I'm not familiar with) and their associated element. This way, hopefully the ambiguity/diversity of atom name formatting gets accurately mapped to the element symbol and subsequently can be mapped to the element's atomic number/vdw radius/full name/ mass (info held within the elements dictionary).

There are some entries relating to course grained simulations and other non-standard elements etc. I would be in favor of us moving those out into a separate backup dictionary perhaps that can be used as a fallback, to help with organisation.

Additionally, there are a whole bunch of 100 values in the 'vdw_radii' keys in the elements subdictionaries, which I suspect are just place-holders. Should a default vdw_radii value be set, for visualization purposes, that is overwritten if an atom has an element with a 'vdw_radii' key in its elements dictionary entry?

You are correct that these are currently just placeholders. I just at the time (when I very first started writing this add-on years ago) wither couldn't be bothered or couldn't reliably get values for those elements and so left them as is. Would be in favor of removing them and using just a fallback value.

I have been thinking also of potentially removing the vdw_radii attribute from being added explicitly from being added, and instead turning this dictionary into a lookup table inside of Geometry Nodes that is only used when needed (for spheres, bond calculations etc). I wouldn't be doing that till Blender 4.1 though when they introduce some new nodes for it.

[BradyAJohnston]

Happy to discuss more about potential design implementations as well.

I'm still not super happy with how I internally go about getting the attributes added to the structure. It was a solution that worked at the time and I just haven't revisited it since.

[rbdavid]

Great!

• I'm gonna remove the placeholder vdw_radii values, remove duplicate and non-element entries (stashing them in a backup dict), and add atomic masses to the elements dict.

• For vdw_radii, a default value of 100 picometers can be used in line 185 of

  MolecularNodes/molecularnodes/io/parse/mda.py

  Lines 180 to 185 in f3dbff0

  	def vdw_radii(self) -> np.ndarray:
  	# pm to Angstrom
  	return np.array(
  	[data.elements.get(element,
  	data.elements.get('X'))
  	.get('vdw_radii') for element in self.elements]) * 0.01 * self.world_scale

  similar to what's done in

  MolecularNodes/molecularnodes/io/parse/molecule.py

  Lines 438 to 445 in f3dbff0

  	def att_vdw_radii():
  	vdw_radii = np.array(list(map(
  	# divide by 100 to convert from picometres to angstroms which is what all of coordinates are in
  	lambda x: data.elements.get(
  	x, {'vdw_radii': 100}).get('vdw_radii', 100) / 100,
  	np.char.title(array.element)
  	)))
  	return vdw_radii * world_scale

  .

• I'll implement a function in both molecule.py and mda.py to gather the atomic masses into a numpy array, mirroring the functions used for vdw_radii.

Towards this last task and

I'm still not super happy with how I internally go about getting the attributes added to the structure. It was a solution that worked at the time and I just haven't revisited it since.

What's wrong with using bpy.data.objects[MN_object_name].data.attributes[attribute_name] and then converting it to a numpy array or mathutils matrix?

Finally, are there unit/integration tests for data.elements or instances where changes to data.elements will cause tests to fail? I haven't done too much digging into the MolecularNodes/tests directory.

[BradyAJohnston]

Finally, are there unit/integration tests for data.elements or instances where changes to data.elements will cause tests to fail? I haven't done too much digging into the MolecularNodes/tests directory.

I haven't got any tests specfically for the data.py files, but they are used for most of the other tests. If you'd like to add some tests specifically for them, I won't say no to more robust tests.

What's wrong with using bpy.data.objects[MN_object_name].data.attributes[attribute_name] and then converting it to a numpy array or mathutils matrix?

I am unsure what you are saying here. I have happy with the mn.blender.obj.set_attribute() and mn.blender.obj.get_attribuet() which get and set attributes from meshes, these take and return numpy arrays.

What I mean more specifically is that I set up all the potential attributes as a function (for the example of the vdw_radii, att_vdw_radii(), which is only evaluated when the function is called. This way I can delay the creation of attribute into a simple loop with try except in case the attribute can't be generated. I am unsure what would be a better approach, of there even would be a better approach, but it feels a bit messy for my liking.

[rbdavid]

Yeah, thinking about this more, the data.elements dictionary is used whenever a structure is loaded - so basically every test is potentially affected by the changes in that dictionary, albeit not likely resulting in any fatal errors. The most precarious type of changes being made...

I don't think a test handle needs to be written around the data.py data structures directly. I guess a better question for me to ask is, how do I run all or a subset of the tests before doing the PR? My previous experience has been using a series of test handles stashed in module files in if __name__ == '__main__': blocks. Then, just run python module.py in a terminal and check the output via assert statements. Is it a similar process here, just in the blender python interpreter? Maybe I'm being overly cautious here though.

[BradyAJohnston]

The tests I run outside of Blender, utlising the bpy module that is installable from pip. You can run the test suite inside of Blender but it can be a bit of a mess around.

Running these inside of the MolecularNodes folder.

conda create -n molecularnodes python==3.10
conda activate molecularnodes
pip install -e .
pip install pytest pytest-snapshot pytest-cov scipy

To run run tests:

# run all tests with verbose output
pytest -v

# run pattern matched tests for a string
pytest -v -k attribute

# run tests in a single file
pytest -v tests/test_attribute.py

[BradyAJohnston]

Apologies as I do need formalise all of this better in a CONTRIBUTING.md

[rbdavid]

I've got code for calculating the Center of Mass but I'm not sure where to place it in the module files. Should users be given the choice to center by the CoG or CoM in the Scene -> Molecular Nodes -> import options?

Here's the code block though:

def center_by_CoM( positions, masses): 
    """
    :parameter positions: np.ndarray, shape = (nAtoms, 3), cartesian coordinates of the structure(s), assumed units of \AA
    :parameter masses: np.ndarray, shape = (nAtoms,), atomic masses of atoms in the structure(s), assumed units of daltons 
                                    and assumed same order as positions
    :return centered_positions: np.ndarray, shape = (nAtoms,3), CoM translation removed from the coordinates
    """
    return positions - np.sum(masses[:,None] * positions, axis = 0) / np.sum(masses)

[rbdavid]

bpy.types.Scene.MN_center_type = bpy.props.EnumProperty(
    name="Method",
    items=(
        ('mass', "Mass", "Adjust the centre of mass to be at the world origin.", 0),
        ('centroid', "Centroid", "Adjust the centroid (ignoring mass) to be att he world origin.", 1)
    )
)

Then display that property on each of the panels for the import methods.

I don't have a great sense for what this will look like in the UI. Is it going to be a single check box for the centre option that then unlocks a choice for mass or centroid via a drop-down menu or further checkboxes?

[rbdavid]

MN_import_centre is defined in ~/molecularnodes/props.py. I assume that's where the MN_center_type, suggested above, would be defined.

Then, in wwpdb.py and local.py, a few chunks of codes need to be updated.

• The def panel(layout, scene): functions need to have grid.prop(scene, 'MN_center_type') added.
• Add a centre_type keyword argument to the fetch() function (

  MolecularNodes/molecularnodes/io/wwpdb.py

  Lines 7 to 40 in f3dbff0

  	def fetch(
  	pdb_code,
  	style='spheres',
  	centre=False,
  	del_solvent=True,
  	cache_dir=None,
  	build_assembly=False,
  	format="mmtf"
  	):
  	if build_assembly:
  	centre = False
  	file_path = download(code=pdb_code, format=format, cache=cache_dir)
  	parsers = {
  	'mmtf': parse.MMTF,
  	'pdb': parse.PDB,
  	'cif': parse.CIF
  	}
  	molecule = parsers[format](file_path=file_path)
  	model = molecule.create_model(
  	name=pdb_code,
  	centre=centre,
  	style=style,
  	del_solvent=del_solvent,
  	build_assembly=build_assembly
  	)
  	model.mn['pdb_code'] = pdb_code
  	model.mn['molecule_type'] = 'pdb'
  	return molecule

  ) and how that function is implemented (

  MolecularNodes/molecularnodes/io/wwpdb.py

  Lines 94 to 102 in f3dbff0

  	mol = fetch(
  	pdb_code=pdb_code,
  	centre=scene.MN_import_centre,
  	del_solvent=scene.MN_import_del_solvent,
  	style=style,
  	cache_dir=cache_dir,
  	build_assembly=scene.MN_import_build_assembly,
  	format=scene.MN_import_format_download
  	)

  ).
• Do similar for the load() function in

  MolecularNodes/molecularnodes/io/local.py

  Lines 22 to 55 in f3dbff0

  	def load(
  	file_path,
  	name="Name",
  	centre=False,
  	del_solvent=True,
  	style='spheres',
  	build_assembly=False
  	):
  	suffix = Path(file_path).suffix
  	parser = {
  	'.pdb': parse.PDB,
  	'.pdbx': parse.CIF,
  	'.cif': parse.CIF,
  	'.mmtf': parse.MMTF,
  	'.bcif': parse.BCIF,
  	'.mol': parse.SDF,
  	'.sdf': parse.SDF
  	}
  	if suffix not in parser:
  	raise ValueError(
  	f"Unable to open local file. Format '{suffix}' not supported.")
  	molecule = parser[suffix](file_path)
  	molecule.create_model(
  	name=name,
  	style=style,
  	build_assembly=build_assembly,
  	centre=centre,
  	del_solvent=del_solvent
  	)
  	return molecule

  and

  MolecularNodes/molecularnodes/io/local.py

  Lines 78 to 86 in f3dbff0

  	mol = load(
  	file_path=file_path,
  	name=scene.MN_import_local_name,
  	centre=scene.MN_import_centre,
  	del_solvent=scene.MN_import_del_solvent,
  	style=style,
  	build_assembly=scene.MN_import_build_assembly,
  	)

  .

Then I'll need to update the Molecule class' create_model() method (

MolecularNodes/molecularnodes/io/parse/molecule.py

Lines 176 to 262 in f3dbff0

	def create_model(
	self,
	name: str = 'NewMolecule',
	style: str = 'spheres',
	selection: np.ndarray = None,
	build_assembly=False,
	centre: bool = False,
	del_solvent: bool = True,
	collection=None,
	verbose: bool = False,
	) -> bpy.types.Object:
	"""
	Create a 3D model of the molecule inside of Blender.
	Creates a 3D model with one vertex per atom, and one edge per bond. Each vertex
	is given attributes which correspond to the atomic data such as `atomic_number` for
	the element and `res_name` for the residue name that the atom is associated with.
	If multiple conformations of the structure are detected, the collection attribute
	is also created which will store an object for each conformation, so that the
	object can interpolate between those conformations.
	Parameters
	----------
	name : str, optional
	The name of the model. Default is 'NewMolecule'.
	style : str, optional
	The style of the model. Default is 'spheres'.
	selection : np.ndarray, optional
	The selection of atoms to include in the model. Default is None.
	build_assembly : bool, optional
	Whether to build the biological assembly. Default is False.
	centre : bool, optional
	Whether to center the model in the scene. Default is False.
	del_solvent : bool, optional
	Whether to delete solvent molecules. Default is True.
	collection : str, optional
	The collection to add the model to. Default is None.
	verbose : bool, optional
	Whether to print verbose output. Default is False.
	Returns
	-------
	bpy.types.Object
	The created 3D model, as an object in the 3D scene.
	"""
	from biotite import InvalidFileError
	if selection:
	array = self.array[selection]
	else:
	array = self.array
	model, frames = _create_model(
	array=array,
	name=name,
	centre=centre,
	del_solvent=del_solvent,
	style=style,
	collection=collection,
	verbose=verbose,
	)
	if style:
	bl.nodes.create_starting_node_tree(
	object=model,
	coll_frames=frames,
	style=style,
	)
	try:
	model['entity_ids'] = self.entity_ids
	except AttributeError:
	model['entity_ids'] = None
	try:
	model['biological_assemblies'] = self.assemblies()
	except InvalidFileError:
	pass
	if build_assembly and style:
	bl.nodes.assembly_insert(model)
	self.object = model
	self.frames = frames
	return model

) and the _create_model function (

MolecularNodes/molecularnodes/io/parse/molecule.py

Lines 321 to 606 in f3dbff0

	def _create_model(array,
	name=None,
	centre=False,
	del_solvent=False,
	style='spherers',
	collection=None,
	world_scale=0.01,
	verbose=False
	) -> (bpy.types.Object, bpy.types.Collection):
	import biotite.structure as struc
	frames = None
	if isinstance(array, struc.AtomArrayStack):
	if array.stack_depth() > 1:
	frames = array
	array = array[0]
	# remove the solvent from the structure if requested
	if del_solvent:
	array = array[np.invert(struc.filter_solvent(array))]
	locations = array.coord * world_scale
	centroid = np.array([0, 0, 0])
	if centre:
	centroid = struc.centroid(array) * world_scale
	# subtract the centroid from all of the positions to localise the molecule on the world origin
	if centre:
	locations = locations - centroid
	if not collection:
	collection = bl.coll.mn()
	bonds_array = []
	bond_idx = []
	if array.bonds:
	bonds_array = array.bonds.as_array()
	bond_idx = bonds_array[:, [0, 1]]
	# the .copy(order = 'C') is to fix a weird ordering issue with the resulting array
	bond_types = bonds_array[:, 2].copy(order='C')
	mol = bl.obj.create_object(name=name, collection=collection,
	vertices=locations, edges=bond_idx)
	# Add information about the bond types to the model on the edge domain
	# Bond types: 'ANY' = 0, 'SINGLE' = 1, 'DOUBLE' = 2, 'TRIPLE' = 3, 'QUADRUPLE' = 4
	# 'AROMATIC_SINGLE' = 5, 'AROMATIC_DOUBLE' = 6, 'AROMATIC_TRIPLE' = 7
	# https://www.biotite-python.org/apidoc/biotite.structure.BondType.html#biotite.structure.BondType
	if array.bonds:
	bl.obj.set_attribute(mol, name='bond_type', data=bond_types,
	type="INT", domain="EDGE")
	# The attributes for the model are initially defined as single-use functions. This allows
	# for a loop that attempts to add each attibute by calling the function. Only during this
	# loop will the call fail if the attribute isn't accessible, and the warning is reported
	# there rather than setting up a try: except: for each individual attribute which makes
	# some really messy code.
	# I still don't like this as an implementation, and welcome any cleaner approaches that
	# anybody might have.
	def att_atomic_number():
	atomic_number = np.array([
	data.elements.get(x, {'atomic_number': -1}).get("atomic_number")
	for x in np.char.title(array.element)
	])
	return atomic_number
	def att_atom_id():
	return array.atom_id
	def att_res_id():
	return array.res_id
	def att_res_name():
	other_res = []
	counter = 0
	id_counter = -1
	res_names = array.res_name
	res_ids = array.res_id
	res_nums = []
	for name in res_names:
	res_num = data.residues.get(
	name, {'res_name_num': -1}).get('res_name_num')
	if res_num == 9999:
	if res_names[counter - 1] != name or res_ids[counter] != res_ids[counter - 1]:
	id_counter += 1
	unique_res_name = str(id_counter + 100) + "_" + str(name)
	other_res.append(unique_res_name)
	num = np.where(np.isin(np.unique(other_res), unique_res_name))[
	0][0] + 100
	res_nums.append(num)
	else:
	res_nums.append(res_num)
	counter += 1
	mol['ligands'] = np.unique(other_res)
	return np.array(res_nums)
	def att_chain_id():
	return np.unique(array.chain_id, return_inverse=True)[1]
	def att_entity_id():
	return array.entity_id
	def att_b_factor():
	return array.b_factor
	def att_occupancy():
	return array.occupancy
	def att_vdw_radii():
	vdw_radii = np.array(list(map(
	# divide by 100 to convert from picometres to angstroms which is what all of coordinates are in
	lambda x: data.elements.get(
	x, {'vdw_radii': 100}).get('vdw_radii', 100) / 100,
	np.char.title(array.element)
	)))
	return vdw_radii * world_scale
	def att_atom_name():
	atom_name = np.array(list(map(
	lambda x: data.atom_names.get(x, -1),
	array.atom_name
	)))
	return atom_name
	def att_lipophobicity():
	lipo = np.array(list(map(
	lambda x, y: data.lipophobicity.get(x, {"0": 0}).get(y, 0),
	array.res_name, array.atom_name
	)))
	return lipo
	def att_charge():
	charge = np.array(list(map(
	lambda x, y: data.atom_charge.get(x, {"0": 0}).get(y, 0),
	array.res_name, array.atom_name
	)))
	return charge
	def att_color():
	return color.color_chains(att_atomic_number(), att_chain_id())
	def att_is_alpha():
	return np.isin(array.atom_name, 'CA')
	def att_is_solvent():
	return struc.filter_solvent(array)
	def att_is_backbone():
	"""
	Get the atoms that appear in peptide backbone or nucleic acid phosphate backbones.
	Filter differs from the Biotite's `struc.filter_peptide_backbone()` in that this
	includes the peptide backbone oxygen atom, which biotite excludes. Additionally
	this selection also includes all of the atoms from the ribose in nucleic acids,
	and the other phosphate oxygens.
	"""
	backbone_atom_names = [
	'N', 'C', 'CA', 'O', # peptide backbone atoms
	"P", "O5'", "C5'", "C4'", "C3'", "O3'", # 'continuous' nucleic backbone atoms
	"O1P", "OP1", "O2P", "OP2", # alternative names for phosphate O's
	"O4'", "C1'", "C2'", "O2'" # remaining ribose atoms
	]
	is_backbone = np.logical_and(
	np.isin(array.atom_name, backbone_atom_names),
	np.logical_not(struc.filter_solvent(array))
	)
	return is_backbone
	def att_is_nucleic():
	return struc.filter_nucleotides(array)
	def att_is_peptide():
	aa = struc.filter_amino_acids(array)
	con_aa = struc.filter_canonical_amino_acids(array)
	return aa | con_aa
	def att_is_hetero():
	return array.hetero
	def att_is_carb():
	return struc.filter_carbohydrates(array)
	def att_sec_struct():
	return array.sec_struct
	# these are all of the attributes that will be added to the structure
	# TODO add capcity for selection of particular attributes to include / not include to potentially
	# boost performance, unsure if actually a good idea of not. Need to do some testing.
	attributes = (
	{'name': 'res_id', 'value': att_res_id,
	'type': 'INT', 'domain': 'POINT'},
	{'name': 'res_name', 'value': att_res_name,
	'type': 'INT', 'domain': 'POINT'},
	{'name': 'atomic_number', 'value': att_atomic_number,
	'type': 'INT', 'domain': 'POINT'},
	{'name': 'b_factor', 'value': att_b_factor,
	'type': 'FLOAT', 'domain': 'POINT'},
	{'name': 'occupancy', 'value': att_occupancy,
	'type': 'FLOAT', 'domain': 'POINT'},
	{'name': 'vdw_radii', 'value': att_vdw_radii,
	'type': 'FLOAT', 'domain': 'POINT'},
	{'name': 'chain_id', 'value': att_chain_id,
	'type': 'INT', 'domain': 'POINT'},
	{'name': 'entity_id', 'value': att_entity_id,
	'type': 'INT', 'domain': 'POINT'},
	{'name': 'atom_id', 'value': att_atom_id,
	'type': 'INT', 'domain': 'POINT'},
	{'name': 'atom_name', 'value': att_atom_name,
	'type': 'INT', 'domain': 'POINT'},
	{'name': 'lipophobicity', 'value': att_lipophobicity,
	'type': 'FLOAT', 'domain': 'POINT'},
	{'name': 'charge', 'value': att_charge,
	'type': 'FLOAT', 'domain': 'POINT'},
	{'name': 'Color', 'value': att_color,
	'type': 'FLOAT_COLOR', 'domain': 'POINT'},
	{'name': 'is_backbone', 'value': att_is_backbone,
	'type': 'BOOLEAN', 'domain': 'POINT'},
	{'name': 'is_alpha_carbon', 'value': att_is_alpha,
	'type': 'BOOLEAN', 'domain': 'POINT'},
	{'name': 'is_solvent', 'value': att_is_solvent,
	'type': 'BOOLEAN', 'domain': 'POINT'},
	{'name': 'is_nucleic', 'value': att_is_nucleic,
	'type': 'BOOLEAN', 'domain': 'POINT'},
	{'name': 'is_peptide', 'value': att_is_peptide,
	'type': 'BOOLEAN', 'domain': 'POINT'},
	{'name': 'is_hetero', 'value': att_is_hetero,
	'type': 'BOOLEAN', 'domain': 'POINT'},
	{'name': 'is_carb', 'value': att_is_carb,
	'type': 'BOOLEAN', 'domain': 'POINT'},
	{'name': 'sec_struct', 'value': att_sec_struct,
	'type': 'INT', 'domain': 'POINT'}
	)
	# assign the attributes to the object
	for att in attributes:
	if verbose:
	start = time.process_time()
	try:
	bl.obj.set_attribute(mol, name=att['name'], data=att['value'](
	), type=att['type'], domain=att['domain'])
	if verbose:
	print(
	f'Added {att["name"]} after {time.process_time() - start} s')
	except:
	if verbose:
	warnings.warn(f"Unable to add attribute: {att['name']}")
	print(
	f'Failed adding {att["name"]} after {time.process_time() - start} s')
	coll_frames = None
	if frames:
	coll_frames = bl.coll.frames(mol.name, parent=bl.coll.data())
	for i, frame in enumerate(frames):
	frame = bl.obj.create_object(
	name=mol.name + '_frame_' + str(i),
	collection=coll_frames,
	vertices=frame.coord * world_scale - centroid
	)
	# TODO if update_attribute
	# bl.obj.set_attribute(attribute)
	mol.mn['molcule_type'] = 'pdb'
	# add custom properties to the actual blender object, such as number of chains, biological assemblies etc
	# currently biological assemblies can be problematic to holding off on doing that
	try:
	mol['chain_ids'] = list(np.unique(array.chain_id))
	except AttributeError:
	mol['chain_ids'] = None
	warnings.warn('No chain information detected.')
	return mol, coll_frames

). Add in a centre_type keyword argument to both.

In the _create_model() function, the

MolecularNodes/molecularnodes/io/parse/molecule.py

Lines 344 to 350 in f3dbff0

	centroid = np.array([0, 0, 0])
	if centre:
	centroid = struc.centroid(array) * world_scale
	# subtract the centroid from all of the positions to localise the molecule on the world origin
	if centre:
	locations = locations - centroid

, do a check to see if centre and centre_type == 'centroid'. If true, run through the code as normal, removing the centroid from the locations. Otherwise, if centre and centre_type == 'mass', need to wait to center the structure until after all attributes are looped over and added to the blender object (line 568). Once atom attributes have been created for the MN object's atoms, then the elements dictionary can be used to map that atomic_number array to an atomic_mass array, which can then be thrown into the code above to calculate the CoM. Remove the CoM from locations and update the position attribute.

[rbdavid]

Log file from the pytest -v is at https://github.com/rbdavid/MolecularNodes/blob/main/tests_results.txt. I haven't done any digging about the various failures. There wouldn't be a log of test results from the original 4.0.11 release, would there?

[BradyAJohnston]

All of the logic looks good that you were laying out above. Instead of adding another attribute, we could change centre to take an enum, with default to None meaning no centering. I think that would be better.

[BradyAJohnston]

For the test result failures - for now run pytest --snapshot-update and it will update the failing snapshots with their new values. Where there are snapshots that say The selected attribute 'entity_id' does not exist on the mesh. Possible attributes are: attribute_names=['b_factor',... It makes sense that they would fail and change - but I should update them in the future that they wouldn't change with the addition of another attribute. Might be something that I tweak in another PR and then you work from that if you'd prefer (and would result in a smaller diff for your PR).

[rbdavid]

Yeah, I'd rather not be the one who pushes the pytest --snapshot-update since I haven't done any work on verifying previous and current tests.

[rbdavid]

bpy.types.Scene.MN_center_type = bpy.props.EnumProperty(
    name="Method",
    items=(
        ('mass', "Mass", "Adjust the centre of mass to be at the world origin.", 0),
        ('centroid', "Centroid", "Adjust the centroid (ignoring mass) to be att he world origin.", 1)
    )
)

So, if I want to add the default value of None to this enum, just do default=None, within that chunk of code? Or do I also need to add a third item to the items tuple?

Something like
('', 'None', 'Do not centre the structure on the world origin.', _some_value_) where maybe the _some_value_ should be zero and the other two items should be shifted by 1? I have zero experience making a GUI panel for Blender.

[BradyAJohnston]

I just merged #447 which should remove most of the failed snapshot tests. Those that now fail should be legitimate failures.

[BradyAJohnston]

So, if I want to add the default value of None to this enum, just do default=None, within that chunk of code? Or do I also need to add a third item to the items tuple?

The enum property types can only take strings, so yes you could supply '' as an option - but it might be better from a UX to keep the tickbox for centering. If unticked then the type is grayed out, if ticked then the user can specify the type of centering.

It would look like the current option for adding a style, which can be disabled, but when enabled provides a dropdown menu (which is how the enum is displayed) to choose the type of style.

MolecularNodes/molecularnodes/io/wwpdb.py

Lines 132 to 135 in 5ac81da

	row.prop(scene, 'MN_import_node_setup', text="")
	col = row.column()
	col.prop(scene, "MN_import_style")
	col.enabled = scene.MN_import_node_setup

[rbdavid]

Yeah, that's perfect! I'll get that implemented. Other than that and tests, I'm not sure if much more is needed. Should I make a PR or wait for word from you?