Collapse classes for Molecule #766
Conversation
BradyAJohnston
changed the title
| @@ -23,6 +23,7 @@ def __init__(self) -> None: | |||
| super().__init__(obj=None) | |||
| self._entity_type: EntityType | |||
| self._register_with_session() | |||
| self._world_scale = 0.01 | |||
There was a problem hiding this comment.
One other non-decanted attribute I had encountered is _assemblies. Not sure if it's needed. But if it is it mid the nice to declare up top
| from ... import color | ||
|
|
||
|
|
||
| class ReaderBase(metaclass=ABCMeta): |
| if isinstance(file_path, str): | ||
| file_path = Path(file_path) | ||
|
|
||
| match file_path.suffix: |
| """ | ||
| return self._code | ||
|
|
||
| def _read(self, file_path: str | Path | io.BytesIO) -> None: |
There was a problem hiding this comment.
Question: why not just load in the data? E.g. instead of returning a handle, why not just create the AtomArray with the fields of interest. You could optionally set the handle as well. Seems like an extra bit of indirection
| super().__init__() | ||
| self._read(file_path) | ||
| self.atom_array: AtomArray | AtomArrayStack | ||
| self._create_object() |
There was a problem hiding this comment.
I think you are using self.tree in other locations? Maybe it would be helpful to declare it here as well.
There was a problem hiding this comment.
whoops. no you are using self.object
| mol._code = code | ||
|
|
||
| if centre: | ||
| mol.position + -mol.centroid(centre) |
There was a problem hiding this comment.
might want to declare mol.position as an attribute at top.
Otherwise it might be mol.object.postion?
| @@ -338,7 +333,7 @@ def assemblies(self, as_array=False): | |||
| transformation matrices, or None if no assemblies are available. | |||
| """ | |||
| try: | |||
| assemblies_info = self._assemblies() | |||
| assemblies_info = self._assemblies | |||
There was a problem hiding this comment.
Maybe add _assemblies as a top level attribute in Init.
tests/test_nodes.py
Outdated
| @@ -64,7 +65,8 @@ def test_selection_working(snapshot_custom: NumpySnapshotExtension, attribute, c | |||
| @pytest.mark.parametrize("code", codes) | |||
| @pytest.mark.parametrize("attribute", ["chain_id", "entity_id"]) | |||
| def test_color_custom(snapshot_custom: NumpySnapshotExtension, code, attribute): | |||
| mol = mn.entities.fetch(code, style="ribbon", cache_dir=data_dir) | |||
| mol = mn.Molecule.fetch(code, cache=data_dir) | |||
| mol.add_style("ribbon") | |||
There was a problem hiding this comment.
I prefer Set style to Add_style . Add_style implies more than one style.
There was a problem hiding this comment.
I'm going with add_style() so that we can add multiple styles on the same Molecule, which will just be additional branches in the node tree. Currently calling add_style() once will be equivalent to the previous style setyp.
returned to filtering out solvent when creating molecule for now to match test snapshots
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #766 +/- ##
==========================================
- Coverage 76.21% 75.75% -0.46%
==========================================
Files 78 77 -1
Lines 5297 5235 -62
==========================================
- Hits 4037 3966 -71
- Misses 1260 1269 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Collapses classes for different data formats that are all now handled by the
Moleculeclass.Simplifies much of the attribute creation logic and code. All potential attributes are initially computed when reading in the file, and set as annotations on the
AtomArray.Then there is just a generic step where all numeric annotations are stored on the mesh as named attributes. This additionally then supports any generic numerical attributes into the future.
New import methods currently don't support: