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.

Sign up for GitHub

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

Jump to bottom

ENH: Rotation.concatenate #14524

Merged
merged 5 commits into from
Aug 4, 2021
Merged

ENH: Rotation.concatenate #14524

merged 5 commits into from
Aug 4, 2021

Conversation

pmla
Copy link
Contributor

@pmla pmla commented Aug 2, 2021

Reference issue

Closes #14340

What does this implement/fix?

This adds a concatenate method similar to np.concatenate but for Rotation objects.

@nmayorov what do you think?

@pmla pmla added enhancement A new feature or improvement scipy.spatial labels Aug 2, 2021
tupui
tupui reviewed Aug 2, 2021
View reviewed changes
Copy link
Member

@tupui tupui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! My 2 cents since I am not a user of these: can you add a validation error test (pytest.raises(... match=...)) and maybe a version added flag?

tupui
tupui approved these changes Aug 3, 2021
View reviewed changes
Copy link
Member

@tupui tupui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, but I would let somebody who uses this have a second look.

nmayorov
nmayorov reviewed Aug 3, 2021
View reviewed changes
scipy/spatial/transform/rotation.pyx Outdated
-----
.. versionadded:: 1.8.0
"""
if not all([isinstance(x, Rotation) for x in rotations]):
Copy link
Contributor

@nmayorov nmayorov Aug 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that [] won't be necessary and it is even more idiomatic to omit it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Nikolay. Done.

@nmayorov nmayorov merged commit 240d4c4 into scipy:master Aug 4, 2021
@tylerjereddy tylerjereddy added this to the 1.8.0 milestone Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmayorov nmayorov nmayorov left review comments

@tupui tupui tupui approved these changes

Assignees
No one assigned
Labels
enhancement A new feature or improvement scipy.spatial
Projects
None yet
Milestone
1.8.0
Development

Successfully merging this pull request may close these issues.

Initialize Rotation using list or array of Rotations
4 participants
@pmla @nmayorov @tupui @tylerjereddy