BioSimSpace icon indicating copy to clipboard operation
BioSimSpace copied to clipboard

About setting the Sire property

Open xiki-tempula opened this issue 3 years ago • 7 comments

I think this might be that I don't fully understand how to modify the Sire mol so I wonder if I could get some help. Lets say I have a BSS.mol object and I set certain property to True, it will not be the same as True.

mol = BSS.Parameters.openff_unconstrained_2_0_0(
    "c1ccccc1").getMolecule()
mol_edit = mol._sire_object.edit()
mol_edit.setProperty("test", True)
mol._sire_object = mol_edit.commit()
assert mol._sire_object.property("test") == True

However, this assertion will give False

xiki-tempula avatar May 19 '22 10:05 xiki-tempula

This is because the property is of type Sire.Base._Base.BooleanProperty, not bool, so the assertion fails when making an equality comparison between the two. When making the assertion you'll need to call the .value() method on the property to get the underlying value, in this case as a bool type.

assert mol._sire_object.property("test").value() == True

lohedges avatar May 19 '22 10:05 lohedges

@lohedges Thanks. When looking at the code, I see examples like mol_edit.setProperty("test", Sire.Base.wrap(True)), I wonder if this is the recommended practice or just mol_edit.setProperty("test", True) is enough?

xiki-tempula avatar May 19 '22 10:05 xiki-tempula

Currently we need to use the wrap function to correctly convert Python types to their Sire equivalent. Soon this will be unnecessary, as @chryswoods has added functionality to do the conversion for you implicitly within wrap itself. (There will also be far greater flexibility in what can actually be stored as a molecular property.)

lohedges avatar May 19 '22 11:05 lohedges

@lohedges It seems that on my computer that mol_edit.setProperty("test", True) works. So your suggestion is that I should do mol_edit.setProperty("test", Sire.Base.wrap(True)) to avoid any unexpected outcome.

xiki-tempula avatar May 19 '22 12:05 xiki-tempula

Perhaps @chryswoods' changes came in on a more recent merge, I can't quite remember. For now I'd use wrap for consistency with the existing code. I plan on updating things like this bit-by-bit over a period of time.

Cheers.

lohedges avatar May 19 '22 12:05 lohedges

Yes, I think this came in with the SDF parser, which made more use of float and bool properties. I got sufficiently annoyed writing wrap(X) that I automated it ;-)

I agree though with Lester that you should use the older way of working in BioSimSpace code until Lester has had a chance to go through and update things. There is quite a bit of change coming in Sire (in feat_web) that, once implemented in BioSimSpace, will make things much easier.

Part of this is a much-improved search functionality. Thanks for giving detail about what you are trying to do. You have inspired me to work on a way of letting you do searches that include properties, e.g. enabling you (in the future) to write code like

my_mols = mols["property test == True"]

chryswoods avatar May 19 '22 13:05 chryswoods

Yes, a general search by molecular properties would be really useful :+1:

lohedges avatar May 19 '22 15:05 lohedges