-
Notifications
You must be signed in to change notification settings - Fork 67
Add consistent parameter docs to the main functions and napari #542
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
Changes from 2 commits
3b3f5fb
e103092
8418f7d
658c6c4
444dd28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -49,10 +49,10 @@ def main( | |||||||||||||||||||||||||
| plane_directory: Optional[str] = None, | ||||||||||||||||||||||||||
| batch_size: Optional[int] = None, | ||||||||||||||||||||||||||
| torch_device: Optional[str] = None, | ||||||||||||||||||||||||||
| split_ball_xy_size: int = 3, | ||||||||||||||||||||||||||
| split_ball_z_size: int = 3, | ||||||||||||||||||||||||||
| split_ball_xy_size: int = 6, | ||||||||||||||||||||||||||
| split_ball_z_size: int = 15, | ||||||||||||||||||||||||||
| split_ball_overlap_fraction: float = 0.8, | ||||||||||||||||||||||||||
| split_soma_diameter: int = 7, | ||||||||||||||||||||||||||
| n_splitting_iter: int = 10, | ||||||||||||||||||||||||||
| *, | ||||||||||||||||||||||||||
| callback: Optional[Callable[[int], None]] = None, | ||||||||||||||||||||||||||
| ) -> List[Cell]: | ||||||||||||||||||||||||||
|
|
@@ -61,77 +61,81 @@ def main( | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Parameters | ||||||||||||||||||||||||||
| ---------- | ||||||||||||||||||||||||||
| signal_array : numpy.ndarray | ||||||||||||||||||||||||||
| 3D array representing the signal data. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| signal_array : numpy.ndarray or dask array | ||||||||||||||||||||||||||
| 3D array representing the signal data in z, y, x order. | ||||||||||||||||||||||||||
| start_plane : int | ||||||||||||||||||||||||||
| Index of the starting plane for detection. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| First plane index to process (inclusive, to process a subset of the | ||||||||||||||||||||||||||
| data). | ||||||||||||||||||||||||||
| end_plane : int | ||||||||||||||||||||||||||
| Index of the ending plane for detection. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| voxel_sizes : Tuple[float, float, float] | ||||||||||||||||||||||||||
| Tuple of voxel sizes in each dimension (z, y, x). | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Last plane index to process (exclusive, to process a subset of the | ||||||||||||||||||||||||||
| data). | ||||||||||||||||||||||||||
| voxel_sizes : 3-tuple of floats | ||||||||||||||||||||||||||
| Size of your voxels in the z, y, and x dimensions (microns). | ||||||||||||||||||||||||||
| soma_diameter : float | ||||||||||||||||||||||||||
| Diameter of the soma in physical units. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| max_cluster_size : float | ||||||||||||||||||||||||||
| Maximum size of a cluster in physical units. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| The expected in-plane (xy) soma diameter (microns). | ||||||||||||||||||||||||||
| max_cluster_size : int | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| max_cluster_size : int | |
| max_cluster_size : float |
I think, for consistency with typehint at least (don't think it matters)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the sig to int, because it's int everywhere else and it's number of voxels so should be int.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's number of voxels
So that means the docstring should say number of voxels instead of cubic um right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh it is um so I changed it to float everywhere..
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 2d in-plane filtering. | |
| 2d in-plane Laplacian of Gaussian filtering. |
Nitpicky, but may help explain the variable name
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| for all the filters. Tune to maximize memory usage without running | |
| out. Check your GPU/CPU memory to verify it's not full. | |
| for all the filters. For performance-critical applications, tune to maximize memory usage without running | |
| out. Check your GPU/CPU memory to verify it's not full. |
This suggestion to make clear that tuning is not a requirement.
Also, should this default to 1, as suggested by the old docstring (but not the code 😅 )?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added it everywhere. For GPU it does default to 1 in the code!?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| split_ball_xy_size: int | |
| Similar to `ball_xy_size`, except the value to use for the 3d | |
| filter during cluster splitting. | |
| split_ball_z_size: int | |
| Similar to `ball_z_size`, except the value to use for the 3d filter | |
| during cluster splitting. | |
| split_ball_xy_size: float | |
| Similar to `ball_xy_size`, except the value (in microns) to use for the 3d | |
| filter during cluster splitting. | |
| split_ball_z_size: float | |
| Similar to `ball_z_size`, except the value (in microns) to use for the 3d filter | |
| during cluster splitting. |
Should now also be floats, as we use them as micron quantities now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed everywhere.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| List of detected potential cells and artifacts. | |
| List of detected cell candidates. |
I think this is the wording we use in the paper and elsewhere.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -80,23 +80,28 @@ class DetectionSettings: | |||||
|
|
||||||
| voxel_sizes: Tuple[float, float, float] = (1.0, 1.0, 1.0) | ||||||
| """ | ||||||
| Tuple of voxel sizes in each dimension (z, y, x). We use this to convert | ||||||
| from `um` to pixel sizes. | ||||||
| Tuple of voxel sizes (microns) in each dimension (z, y, x). We use this | ||||||
| to convert from `um` to pixel sizes. | ||||||
| """ | ||||||
|
|
||||||
| soma_spread_factor: float = 1.4 | ||||||
| """Spread factor for soma size - how much it may stretch in the images.""" | ||||||
| """ | ||||||
| Cell spread factor for determining the largest cell volume before | ||||||
| splitting up cell clusters. Structures with spherical volume of | ||||||
| diameter `soma_spread_factor * soma_diameter` or less will not be | ||||||
| split. | ||||||
| """ | ||||||
|
|
||||||
| soma_diameter_um: float = 16 | ||||||
| """ | ||||||
| Diameter of a typical soma in um. Bright areas larger than this will be | ||||||
| split. | ||||||
| Diameter of a typical soma in-plane (xy) in microns. | ||||||
| """ | ||||||
|
|
||||||
| max_cluster_size_um3: float = 100_000 | ||||||
| """ | ||||||
| Maximum size of a cluster (bright area) that will be processed, in um. | ||||||
| Larger bright areas are skipped as artifacts. | ||||||
| Largest detected cell cluster (in cubic um) where splitting | ||||||
| should be attempted. Clusters above this size will be labeled | ||||||
| as artifacts. | ||||||
| """ | ||||||
|
|
||||||
| ball_xy_size_um: float = 6 | ||||||
|
|
@@ -116,17 +121,21 @@ class DetectionSettings: | |||||
|
|
||||||
| ball_overlap_fraction: float = 0.6 | ||||||
| """ | ||||||
| Fraction of overlap between a bright area and the spherical kernel, | ||||||
| for the area to be considered a single ball. | ||||||
| Fraction of the 3d ball filter needed to be filled by foreground voxels, | ||||||
| centered on a voxel, to retain the voxel. | ||||||
| """ | ||||||
|
|
||||||
| log_sigma_size: float = 0.2 | ||||||
| """Size of the sigma for the 2d Gaussian filter.""" | ||||||
| """ | ||||||
| Gaussian filter width (as a fraction of soma diameter) used during | ||||||
| 2d in-plane filtering. | ||||||
|
||||||
| 2d in-plane filtering. | |
| 2d in-plane Laplacian of Gaussian filtering. |
Consistent with previous suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
presumably this will come in a follow up PR shortly? (it's not currently part of the function signature)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many PRs at once... It leaked over :D