Skip to content

Expose rotation submodule#306

Open
robjmcgibbon wants to merge 1 commit intomasterfrom
import_rotation
Open

Expose rotation submodule#306
robjmcgibbon wants to merge 1 commit intomasterfrom
import_rotation

Conversation

@robjmcgibbon
Copy link
Copy Markdown
Collaborator

Rotations had to be imported explicitly, which I don't think should be required.

In [1]: import swiftsimio as sw
sw.
In [2]: sw.visualisation.rotation
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[2], line 1
----> 1 sw.visualisation.rotation

AttributeError: module 'swiftsimio.visualisation' has no attribute 'rotation'

In [3]: import swiftsimio.visualisation.rotation

@kyleaoman
Copy link
Copy Markdown
Member

Fair enough. On the other hand, I've just had a look and this function is not used internally in swiftsimio itself anywhere. It is used:

  • In the tests for rotated visualisations.
  • In the documentation explaining how to make rotated visualisations.

The purpose is just to get the rotation matrix that aligns a vector with one of the coordinate axes. For that there's already scipy.spatial.transform.Rotation.align_vectors... I wonder if we could just drop this function, or make it a thin wrapper around that... anyway, for what it is this PR seems fine. Whenever you're ready just flag for review.

@robjmcgibbon robjmcgibbon requested a review from kyleaoman April 2, 2026 15:31
@robjmcgibbon
Copy link
Copy Markdown
Collaborator Author

Thanks. I just had someone yesterday coming to ask me why it wasn't working, so I don't think we should get rid of it since some people must be relying on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants