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

[DOC] clarify that the inertia_tensor may be nD in documentation #4013

Merged

Conversation

marshuang80
Copy link
Contributor

Description

Fixed import issue and documentation for inertia_tensor #4004

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.
  • Consider backporting the PR with @meeseeksdev backport to v0.14.x

Copy link
Member

@alexdesiqueira alexdesiqueira left a comment

Choose a reason for hiding this comment

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

Thank you very much for tackling this, @marshuang80! I left some comments already 😅

@@ -657,10 +657,10 @@ def regionprops(label_image, intensity_image=None, cache=True):
bounding box.
**image** : (H, J) ndarray
Sliced binary region image which has the same size as bounding box.
**inertia_tensor** : (2, 2) ndarray
**inertia_tensor** : (Ndim, Ndim) ndarray
Copy link
Member

Choose a reason for hiding this comment

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

I think only mentioning this as ndarray should be sufficient:
**inertia_tensor** : ndarray

Inertia tensor of the region for the rotation around its mass.
**inertia_tensor_eigvals** : tuple
The two eigen values of the inertia tensor in decreasing order.
The N eigen values of the inertia tensor in decreasing order.
Copy link
Member

Choose a reason for hiding this comment

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

The N here is not necessary. It could be:
The eigenvalues of the inertia tensor in decreasing order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

Copy link
Member

@alexdesiqueira alexdesiqueira left a comment

Choose a reason for hiding this comment

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

Thank you @marshuang80!

@alexdesiqueira alexdesiqueira requested a review from jni July 13, 2019 17:04
@hmaarrfk hmaarrfk changed the title [FIX] inertia tensor documentation [DOC] clarify that the inertia_tensor may be nD in documentation Jul 13, 2019
@hmaarrfk hmaarrfk merged commit 3049a03 into scikit-image:master Jul 13, 2019
@hmaarrfk
Copy link
Member

Thanks!

@marshuang80 marshuang80 deleted the inertia-tensor-documentation branch July 13, 2019 17:11
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.

None yet

3 participants