Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] make_whole center kwarg not exposed #199

Closed
lohedges opened this issue Jun 6, 2024 · 1 comment · Fixed by #206
Closed

[BUG] make_whole center kwarg not exposed #199

lohedges opened this issue Jun 6, 2024 · 1 comment · Fixed by #206
Assignees
Labels
bug Something isn't working
Milestone

Comments

@lohedges
Copy link
Contributor

lohedges commented Jun 6, 2024

The make_whole method on a cursor implies that that there is a center kwarg that can be used to optionally re-center the system when unwrapping molecules that are split across the periodic boundary. However, this only appears to be applied to the Cursor base class, not, e.g. CursorsM. In this case, it needs to be passed as a positional argument:

In [1]: import sire as sr

In [2]: mols = sr.load_test_files("ala.crd", "ala.top")

In [3]: center = mols[0].coordinates()

In [4]: mols.make_whole(center=center)
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ in <module>:1                                                                                    │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
TypeError: System.make_whole() got an unexpected keyword argument 'center'

In [5]: mols.make_whole(center)
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ in <module>:1                                                                                    │
│                                                                                                  │
│ /home/lester/.conda/envs/openbiosim/lib/python3.12/site-packages/sire/system/_system.py:159 in   │
│ make_whole                                                                                       │
│                                                                                                  │
│    156 │   │   else:                                                                             │
│    157 │   │   │   from ..base import create_map                                                 │
│    158 │   │   │                                                                                 │
│ ❱  159 │   │   │   self._system.make_whole(map=create_map(map))                                  │
│    160 │   │                                                                                     │
│    161 │   │   self._molecules = None                                                            │
│    162                                                                                           │
│                                                                                                  │
│ /home/lester/.conda/envs/openbiosim/lib/python3.12/site-packages/sire/base/__init__.py:201 in    │
│ create_map                                                                                       │
│                                                                                                  │
│   198if map is None:                                                                        │
│   199 │   │   wrapped_values = {}                                                                │
│   200 │   │                                                                                      │
│ ❱ 201 │   │   for key, value in values.items():                                                  │
│   202 │   │   │   try:                                                                           │
│   203 │   │   │   │   wrapped_values[key] = _Base.PropertyName(value)                            │
│   204 │   │   │   except Exception:                                                              │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
AttributeError: 'Vector' object has no attribute 'items'

In [6]: cursor = mols.cursor()

In [7]: cursor = cursor.make_whole(center=center)
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ in <module>:1                                                                                    │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
TypeError: CursorsM.make_whole() got an unexpected keyword argument 'center'

In [8]: cursor = cursor.make_whole(center)

It would also be nice if the center kwarg was exposed when calling make_whole on the sire.system._system.System object.

@lohedges lohedges added the bug Something isn't working label Jun 6, 2024
@chryswoods
Copy link
Contributor

This should be a quick fix - I'll work on this next for the 2024.2.0 release.

@chryswoods chryswoods added this to the 2024.2.0 milestone Jun 22, 2024
@chryswoods chryswoods self-assigned this Jun 22, 2024
@chryswoods chryswoods mentioned this issue Jun 23, 2024
@chryswoods chryswoods linked a pull request Jun 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants