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

Profile line for 3d array added to profile.py #1884

Open
wants to merge 39 commits into
base: main
Choose a base branch
from

Conversation

Maxyme
Copy link

@Maxyme Maxyme commented Jan 19, 2016

I added a method to generate a profile line of a 3d array. It samples points around the direction axis, by rotating them along the same axis. The number of sample points is related to the line width. For a line width of 1 no rotation is needed.
Otherwise, the number of angles is generated like this: rotation_angles = np.linspace(0, constants.pi, (linewidth * 2) - 1)

Also, arrays are converted to float before sending to ndi_map_coordinates, because the order value would be ignored when the arrays are int.
This is my first commit, let me know of any changes required. I also assume some functions may be optimized.

@Maxyme
Copy link
Author

Maxyme commented Jan 19, 2016

@Korijn :
Fixed the loop with transposed matrices (for perp_points in perp_array.T: instead of for i in transposed_array.shape[0]).
Fixed the main method with if/elif switch depending on dimension and multichannel instead of first multichannel and then dimension.

perp_cols = np.array([np.linspace(col_i - col_width, col_i + col_width,
linewidth) for col_i in line_col])
perp_plane = np.array([np.linspace(slice_i - slice_width, slice_i + slice_width,
linewidth) for slice_i in line_slice])
Copy link
Member

Choose a reason for hiding this comment

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

We use (plane, row, column) as 3D coordinates, see Coordinate conventions

@jni
Copy link
Member

jni commented Jan 21, 2016

@Maxyme Awesome, thank you for this contribution! It's something that's been on the back of my mind for a long time but my 3D geometry wasn't up to snuff. I've made some stylistic comments and a couple of implementation suggestions. After that I can have another stab at it. Thank you again!

@jni
Copy link
Member

jni commented Jan 21, 2016

To make some of the algebra less hairy, using quaternions might be handy. This stackoverflow answer has a nice, simple implementation that could be used here. Maybe? As I said, my 3D geometry is not great!

@Maxyme
Copy link
Author

Maxyme commented Jan 21, 2016

@jni Thanks for your corrections. I'll have another go at it today. You are correct that the sampling method does generate a denser sampling near the center of the cylinder, or disk. One way of solving this is to reduce the angle size as we move farther from the center of the cylinder. Then, we could have a similar density of sample points.

@stefanv stefanv added the ⏩ type: Enhancement Improve existing features label Jan 22, 2016
Maxime added 2 commits January 22, 2016 10:32
…he distance between the point and the center of the axis increases
rotation_angles = np.array([0])
else:
rotation_angles = np.linspace(0, 2 * constants.pi, 2 * distance_point_line + 3)
rotation_angles = np.delete(rotation_angles, len(rotation_angles) - 1)
Copy link
Member

Choose a reason for hiding this comment

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

Is this not equal to rotation_angles[:-1]?

@jni
Copy link
Member

jni commented Sep 6, 2016

@Maxyme As @ahojnnes, I apologise for the long delay in getting back to you. We are actively trying to improve our process to prevent such long waits in the future.

Do you have time soon to address our comments?

@pep8speaks
Copy link

pep8speaks commented Sep 25, 2018

Hello @Maxyme! Thanks for updating the PR.

Line 162:41: E241 multiple spaces after ','

Comment last updated on October 16, 2018 at 19:33 Hours UTC

@codecov-io
Copy link

codecov-io commented Sep 25, 2018

Codecov Report

Merging #1884 into master will decrease coverage by 0.81%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1884      +/-   ##
=========================================
- Coverage   87.61%   86.8%   -0.82%     
=========================================
  Files         323     341      +18     
  Lines       27138   27454     +316     
=========================================
+ Hits        23777   23831      +54     
- Misses       3361    3623     +262
Impacted Files Coverage Δ
skimage/io/_plugins/imread_plugin.py 69.23% <0%> (-15.39%) ⬇️
skimage/io/tests/test_fits.py 83.01% <0%> (-13.21%) ⬇️
skimage/io/_plugins/fits_plugin.py 81.13% <0%> (-9.44%) ⬇️
skimage/io/tests/test_imread.py 70.58% <0%> (-3.93%) ⬇️
skimage/viewer/tests/test_tools.py 97.34% <0%> (-2.66%) ⬇️
skimage/future/graph/rag.py 95.42% <0%> (-0.66%) ⬇️
skimage/io/manage_plugins.py 87.4% <0%> (-0.57%) ⬇️
skimage/filters/thresholding.py 98.14% <0%> (-0.56%) ⬇️
skimage/restoration/tests/test_denoise.py 98.68% <0%> (-0.27%) ⬇️
skimage/feature/match.py 96.96% <0%> (-0.26%) ⬇️
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6271b1a...ed9eae8. Read the comment docs.


p3 = (c * (u ** 2 + v ** 2) - w * (a * u + b * v - u * p - v * q - w * r)) * \
(1 - np.cos(angle_in_radians)) + r * np.cos(angle_in_radians) + \
(-b * u + a * v - v * p + u * q) * np.sin(angle_in_radians)
Copy link
Member

Choose a reason for hiding this comment

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

Can you provide a reference for this formula?

Copy link
Author

Choose a reason for hiding this comment

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

Hi, thanks for the comment, I'm in the long due process of updating this PR. I will add the reference here.

@stefanv
Copy link
Member

stefanv commented Oct 18, 2018

PR resurrected from the dead! 🧟

@Maxyme
Copy link
Author

Maxyme commented Oct 24, 2018

@stefanv Yes, it's been too long. I've updated it a bit, but I think it still needs a thorough review. There may be duplicate or misplaced chunks of code (linalg, etc) so any suggestion in this regard is appreciated.

Base automatically changed from master to main February 18, 2021 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏩ type: Enhancement Improve existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants