Conversation
Conflicts: src/skimage2/__init__.pyi src/skimage2/meson.build
|
Reviews (1): Last reviewed commit: "Apply commit hooks" | Re-trigger Greptile |
| # skimage2.filters.gaussian ------------------------------------------------------------ | ||
|
|
||
|
|
||
| def test_gaussian_negative_sigma(): |
There was a problem hiding this comment.
This is why I like using classes to group test together.
Without doing that and if I want to be able to use something like pytest -k 'gaussian' effectively, I need to insert the common prefix in every test function name. And I need a comment to separate test groups at a quick glance.
Given all that I don't see what's so bad about having a test class. "We don't do classes anymore" doesn't really seem like a constructive argument that addresses these requirements. 😉
There was a problem hiding this comment.
I use file-based grouping and it works fine? 🤷 ie if you make one file per test class, it becomes exactly equivalent. pytest -k GaussianClass becomes pytest src/skimage2/filters/_tests/test_gaussian.py. I can see that it's more of a pain to type once, but on the other hand, you get autocomplete, so 🤷
|
Reviews (2): Last reviewed commit: "Address oversights" | Re-trigger Greptile |
which is now raised internally by `gaussian`.
| mask = gaussian( | ||
| mask, | ||
| sigma=0.25 * min_length * blob_size_fraction, | ||
| preserve_range=False, | ||
| mode=boundary_mode, | ||
| mask, sigma=0.25 * min_length * blob_size_fraction, mode=boundary_mode | ||
| ) | ||
| threshold = np.quantile(mask, 1 - volume_fraction) | ||
| blobs = mask >= threshold |
There was a problem hiding this comment.
We can just use gaussian from skimage2 here without having to worry about preserve range because the thresholding is insensitive to the value range of the smoothed mask.
|
Reviews (3): Last reviewed commit: "Fix tests that are sensitive to PendingS..." | Re-trigger Greptile |
|
Reviews (4): Last reviewed commit: "Fix warning sensitive test_list_sigma" | Re-trigger Greptile |
and removed ._shared.filters.
|
Reviews (5): Last reviewed commit: "Fix references to moved skimage._shared" | Re-trigger Greptile |
|
Tip: Greploop — Automatically fix all review issues by running Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal. |
Description
Previously,
gaussianlived inskimage/_shared/to avoid import circles. Now that lazy loading is established, I don't think that's a problem any more. So I've put it back inskimage2/filters/. If it becomes one, it's easy enough to move the function back to_shared.In our discussion around
preserve_range, we agreed to drop this keyword. We aim to useprescalewhere the algorithm needs to know about the value range, elsewhere we just explain users how to recover legacy behavior._rescale_value_range(..., mode="legacy")inskimage2to enable users to recover the legacy behavior. I propose to do that in another PR and have marked this one as Draft for now. See Add rescaling API to skimage2 #8075Checklist
./doc/examplesfor new featuresRelease note
For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.