Address inverse-FFT / edge-detection review findings#15
Merged
Conversation
inverse_fft.py: - Move `from dataclasses import replace` to module scope (was imported inside the per-region loop in fourier_region_mask). - Type FourierStrokes.stamps as tuple[tuple[float, float], ...]. edge_detection.py (gradient_filter): - Medium: stop fabricating edges at NaN-region borders. Mean-filling non-finite pixels steps the hole rim, so every pixel whose 3x3 operator footprint touches a NaN had a spurious gradient (~15x baseline) while only the NaN pixels themselves were excluded. Now dilate the NaN mask by the 3x3 footprint and null that band (magnitude->0, orientation->NaN, display->NaN) before normalisation/thresholding, and drop it from the edge mask. - Low: ROI/invalid regions use a NaN orientation sentinel instead of 0.0 (which is a valid +x direction and conflated "no data" with "horizontal"). - Low: document that gradient_orientation is in image coordinates (row axis points down), not the math/physical y-up convention. Adds regression tests: NaN-border exclusion, clean-image no-op, NaN orientation sentinel outside ROI. Full suite green, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fresh review pass over the inverse-FFT and edge-detection modules.
inverse_fft.py (minor)
from dataclasses import replaceout of the per-region loop infourier_region_maskto module scope.FourierStrokes.stampsastuple[tuple[float, float], ...].(Even-N Nyquist conjugate limitation and the O(stamps × pixels) stroke cost were reviewed and left as-is — the former is documented and surfaced via
imag_residual_norm, the latter is fine for typical brush counts.)edge_detection.py —
gradient_filterNaNfor orientation instead of0.0(a valid +x direction that conflated "no data" with "horizontal").gradient_orientationis in image coordinates (row axis points down), noted on both the function andEdgeDetectionResult.Verification
ruff checkclean.🤖 Generated with Claude Code