New Data Structures for HOOMD objects #776
Conversation
Also update all imports, and make TypeParameterDict and AttachedTypeParameterDict inherit from MutableMapping
The method now requires a copy of the object changed. This lets the parent to always have a copy of the object when performing any necessary syncing. This is necessary for the AttachedTypeParameterDict.
Remove callback as well, since it wasn't used
When using OnlyIf or Either, where the internal definition is a data structure we grab the inner validation when creating the data structure.
prevents raising KeyErrors and moves then to AttributeErrors
When an interior data structure becomes no longer connected to its parent, we set the _parent attribute to false.
This does a large amount of modification of the code to deal with the fact that the module structure has changed since this PR was created.
Also, fix ParameterDict and _HOOMDBaseObject to work with data structure updates.
Make data structure classes user facing
b-butler
force-pushed
the
refactor/data-structures
branch
from
1a9fb3d to
712f332
Compare
b-butler
requested review from
a team and
joaander
and removed request for
a team
|
I am having some documentation build warnings to due with |
Use f-strings and change warning regarding instantiation.
The if clause likely did nothing since the preprocessing would make the inclusion check false anyways.
|
I'm happy to provide a review for this in the near future. I should also say that I intend to finish up the work on glotzerlab/signac#249, and tbh I'm wondering if extracting the final version of that into a separate package and reusing it here will be a better solution prior to the final v3 release of HOOMD. Right now this is a ton of logic directly duplicated from the design in signac. I was already considering eventually extracting that into a package, but didn't want to touch on that until it was merged into signac properly; these changes to HOOMD definitely suggest that it might be beneficial to spin that off separately. |
The method is not needed
Also, rename ``self._dict`` to ``self._data`` to match other data structures
Moves initialization logic to ``_ValidatedDefaultDict`` and adds a static method for converting entries in the dict to base Python types with an option to deepcopy other values.
Since we implement ``__getattr__`` for ``HOOMDDict`` and ``TypeParameter``, we have to manually implement ``__getstate__`` and ``__setstate__``, (internal Python thing).
b-butler
force-pushed
the
refactor/data-structures
branch
from
6d24ac0 to
6e0d0e8
Compare
|
There are lots of good discussions on nesting, properties and user expectations in #819. I leave the decision and implementation up to you all. Python does not make this easy. In C++, you know if you have a reference or a copy. |
|
Just an update, I haven't reviewed this yet because I've been putting my time into the signac PR that I mentioned. I think that once the details are worked out there we should be able to copy a lot of it in directly and just implement a couple of small classes defining how HOOMD objects should be synchronized. Once that PR is closer to complete I'll ping @b-butler to discuss details. |
|
@b-butler I will let you handle the merge of master into this branch. Let me know if you have any questions. |
|
Is this functionality we are still planning to complete? Or should we close this PR? |
|
@joaander yes and no, some of these changes were necessary for #944, but others weren't. I know @vyasr thought that using the syncing code he wrote for signac would make this simpler to implement, so I am currently waiting on #944, and then I will probably re-implement the changes using the signac code, but I guess that means I can close this to open another PR later. |
Description
This fleshes out HOOMD's data structures that sync with C++. It adds classes that can inform parents on modification. This allows us to do things like
lj.params[('A', 'A')].epsilon =4and expect it to work correctly.The main caveat here is that upon attaching the data structures may not always be up to date if C++ changes them (in general it shouldn't and this is only for complex structures like lists, dictionaries, and sets). However, the setting of these parameters using the new data structures still works as expected even if it isn't necessarily up to date.
We rely on Python's reference system to only every expose a user to the exact same data structure object to represent the same data. This may seem obvious, but it is required to allow us to break the connection of internal data structures from them parent's to prevent erroneous updates.
Motivation and context
I was motivated by #758 and conversations had with the development team. Since the beta release will be rolling out soon, I thought a more robust system for handling HOOMD object's attributes was in order.
How has this been tested?
Existing tests pass, but more should be added.
Checklist:
sphinx-doc/credits.rst) in the pull request source branch.