aiidateam / aiida-atomistic

AiiDA plugin which contains data and methods for atomistic simulations.
https://aiidateam.github.io/aiida-atomistic/
MIT License
3 stars 7 forks source link

`StructureData.cell` in `dir()` but raises `AttributeError` #12

Open mbercx opened 6 months ago

mbercx commented 6 months ago

I was playing around with the basic StructureData and found some very strange behavior. Running the following:

from aiida_atomistic.data.structure import StructureData

properties_dict = {
    "cell":{"value":[[3.5, 0.0, 0.0], [0.0, 3.5, 0.0], [0.0, 0.0, 3.5]]},
    "pbc":{"value":[True,True,True]},
    "positions":{"value":[[0.0, 0.0, 0.0],[1.5, 1.5, 1.5]]},
    "symbols":{"value":["Li","Li"]},
    }

structure = StructureData(properties = properties_dict)

print('cell' in dir(structure))
structure.cell

prints True, but then raises an AttributeError when I try to access cell. 😅

mikibonacci commented 6 months ago

yes, for now I have not deleted direct attributes like cell, pbc and so on, which are of the old StructureData. But this are no more working and are not related to the properties. I just left them there because I am not sure we really want to put everything under the properties attribute. In that case, the true structuredata node will be contained in the properties attribute, and nothing else is left outside (apart the Data methods)

mbercx commented 6 months ago

I just left them there because I am not sure we really want to put everything under the properties attribute. In that case, the true structuredata node will be contained in the properties attribute, and nothing else is left outside (apart the Data methods)

Yes, this is also an observation I was making, and would make certain operations more cumbersome than they have to be. My first inclination is that I would really like to have structure.cell. But then if we treat all properties equally, all of them should have a corresponding attribute. And maybe that's ok, but then what's the point of the properties namespace?

I'm not even considering the implementation here. Just the API.