Fixed skimage.feature.graycomatrix example documentation. #7297
Conversation
…n matches the example code.
soupault
added
the
📄 type: Documentation
|
Thank you, @FedericoWZhaw! May I ask that you wrap the line at 88 characters? I'm actually surprised that pre-commit passed... scikit-image/.pre-commit-config.yaml Line 35 in f4c1b34 |
Done |
| Compute 2 GLCMs: One for a 1-pixel offset to the right, and one | ||
| for a 1-pixel offset upwards. | ||
| Compute 4 GLCMs at 1-pixel offset for 4 different angles. For example, 0 radians | ||
| refers to the pixel to the right and pi/2 to the pixel above. |
There was a problem hiding this comment.
Thank you, @FedericoWZhaw! I would be a little more explicit; maybe:
Compute 4 GLCMs: The first one for a 1-pixel offset to the right (distance 1 and angle 0 rad); the second one for a 1-pixel offset to the first bisector (distance 1 and angle pi/4); etc.
or include it as comments in the code, when each GLCM is printed. Is it correct that pi/4 and 3*pi/4 correspond to the upward diagonal directions?
There was a problem hiding this comment.
"1-pixel offset" and "distance 1" seem somewhat redundant, I find the later more precise? "bisector" seems like a very technical term that's not used anywhere else in the docstring..?
There was a problem hiding this comment.
"1-pixel offset" and "distance 1" seem somewhat redundant, I find the later more precise?
The description of the distance parameter states:
distances : array_like
List of pixel pair distance offsets.
What about:
"Compute 4 GLCMs using 1-pixel distance and 4 different angles. For example, an angle of 0 radians refers to the neighboring pixel to the right; pi/4 to the top-right diagonal neighbor (row-1,column+1); pi/2 to the pixel above, etc."
Feel free to suggest something else.
There was a problem hiding this comment.
Thank you both for iterating! 😃
"1-pixel offset" and "distance 1" seem somewhat redundant
Totally, but that was on purpose, to clarify the arguments, as done in the text of the gallery example: "a 1-pixel offset to the right (distance 1 and angle 0 rad)" parallels "a horizontal offset of 5 (distance=[5] and angles=[0])."
"bisector" seems like a very technical term
I must say I'm not sure how it sound to most ears... I would assume that someone using scikit-image would have some minimal computing background and, for instance, would have already heard something like "the equation of the first bisector line is y=x" (but I'm definitely not attached to this term).
I very much like @FedericoWZhaw's suggestion (minor: I would write "0 radian" instead of "0 radians"). It's just that -- and that was my original motivation for suggesting a change -- an angle refers to a direction so, e.g., an angle of 0 refers to all pixels to the right, not only to the pixel to the right; the fact that it's the pixel to the right comes from the pair (distance=1, angle=0); same with all other angles (angle pi/2 means one of the pixels straight above, and it's the pixel right above because it's at distance 1).
|
@FedericoWZhaw are you able to wrap this up? Or would you like us to take over and co-author a third/final commit? Thanks! |
Thanks for the inputs and the interesting discussion! Since it is not fully clear to me if we want to go for a variant of : "Compute 4 GLCMs using 1-pixel distance and 4 different angles. For example, an angle of 0 radians refers to the neighboring pixel to the right; pi/4 to the top-right diagonal neighbor (row-1,column+1); pi/2 to the pixel above, etc." or "Compute 4 GLCMs: The first one for a 1-pixel offset to the right (distance 1 and angle 0 rad); the second one for a 1-pixel offset to the first bisector (distance 1 and angle pi/4); etc." I think it's best you take over the final commit, that should be the fastest solution. |
Fixed documentation according to #7293