Improve X-ray docs #578
Improve X-ray docs #578
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #578 +/- ##
==========================================
- Coverage 93.58% 93.52% -0.06%
==========================================
Files 92 92
Lines 6123 6127 +4
==========================================
Hits 5730 5730
- Misses 393 397 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
|
||
| Returns: | ||
| (num_angles, 2, 4) array of homogeneous projection matrices. | ||
|
|
||
| """ | ||
| if angles is not None and vectors is not None: | ||
| raise ValueError("`angles` and `vectors` are mutually exclusive.") |
There was a problem hiding this comment.
Perhaps "... mutually exclusive parameters" for clarity?
| angles: Array of projection angles in radians. | ||
| vectors: Array of geometry specification vectors. | ||
| angles: Array of projection angles in radians, mutually | ||
| exclusive with `vectors`. |
There was a problem hiding this comment.
Perhaps "... radians. This parameter is mutually exclusive with ...", and likewise for the line below?
Also, before "Args:", "These options are ..." would be better as "These parameters are ..."
| @@ -604,12 +620,15 @@ def __init__( | |||
|
|
|||
| if not isinstance(det_count, (list, tuple)) or len(det_count) != 2: | |||
| raise ValueError("Expected det_count to be a tuple with 2 elements.") | |||
| if angles is not None and vectors is not None: | |||
| raise ValueError("`angles` and `vectors` are mutually exclusive.") | |||
There was a problem hiding this comment.
Perhaps "Parameters angles and vectors ..."? Also, exceptions typically go to a text console, so no point in adding markup. (Also applies to other exceptions in this PR.)
| @@ -65,13 +65,18 @@ def _project_coords( | |||
| x_volume: np.ndarray, vol_geom: VolumeGeometry, proj_geom: ProjectionGeometry | |||
| ) -> np.ndarray: | |||
| """ | |||
| Transform volume (logical) coordinates into world coordinates based | |||
| Project volume (logical) coordinates into detector coordinates based | |||
There was a problem hiding this comment.
Is this a change of terminology from "world coordinates" to "detector coordinates"? If so, corresponding changes are required elsewhere for consistency.
Why are volume coordinates "logical"?
| @@ -65,13 +65,18 @@ def _project_coords( | |||
| x_volume: np.ndarray, vol_geom: VolumeGeometry, proj_geom: ProjectionGeometry | |||
| ) -> np.ndarray: | |||
| """ | |||
| Transform volume (logical) coordinates into world coordinates based | |||
| Project volume (logical) coordinates into detector coordinates based | |||
| on ASTRA geometry objects. | |||
|
|
|||
| Args: | |||
| x_volume: (..., 3) vector(s) of volume (AKA logical) coordinates | |||
There was a problem hiding this comment.
AKA is a bit jarring. Perhaps "i.e."?
| and `v` with center `d`. | ||
| and `v` with center `d`. The term ""world"" emphasizes that the | ||
| function is intended to be used on 3D coordinates representing a | ||
| point in physical space, rather than a logical index into the volume |
There was a problem hiding this comment.
"logical" implies an integer index into an array?
| @@ -95,7 +100,10 @@ def project_world_coordinates( | |||
| """Project world coordinates along ray into the specified basis. | |||
|
|
|||
| Project world coordinates along `ray` into the basis described by `u` | |||
| and `v` with center `d`. | |||
| and `v` with center `d`. The term ""world"" emphasizes that the | |||
There was a problem hiding this comment.
Why double quotes on either side of "world"?
The discussion might work better in the module docstring at the beginning of the file. |
Addresses #576. Is there a better place to put the text about world coordinates?