Open
Conversation
- Updated the documentation for the `compute_kronecker_product` method to clarify the expansion of 1D angle functions to N-D arrays, including detailed parameter descriptions and return types. - Refactored the implementation to ensure C-contiguous output and improved handling of tensor shapes during the Kronecker product computation. - Enhanced the PyTorch equivalent in the `torch/_udct_windows.py` file to maintain consistency with the NumPy implementation, ensuring similar functionality and documentation clarity.
- Replaced the `_from_sparse` method with the `SparseWindow` class for handling sparse window representations in UDCT and related files, enhancing code clarity and maintainability. - Updated type annotations across multiple files to reflect the new `SparseWindow` structure, ensuring consistency in handling window data. - Removed obsolete functions and tests related to the previous sparse handling method, streamlining the codebase. - Enhanced documentation to clarify the usage of the `SparseWindow` class and its methods, improving usability for future development.
…sform files - Updated type annotations for the `window` parameter in multiple functions to use the `SparseWindow` class directly, enhancing code clarity. - Reformatted function calls for better readability by breaking long lines into multiple lines, improving maintainability. - Ensured consistency in the handling of `SparseWindow` across various transformation implementations in the NumPy module.
…T examples - Updated type annotations in `plot_02_direction_resolution.py` and `plot_06_monogenic_curvelet.py` to enhance clarity and consistency. - Improved the handling of `SparseWindow` instances across various functions, ensuring better type safety and maintainability. - Refactored the `color_windows` and `plot_disk` functions to utilize updated type hints, enhancing code readability. - Adjusted the test cases in `test_udct_windows_comparison.py` to align with the new SparseWindow structure, ensuring accurate comparisons between NumPy and PyTorch implementations.
…rameters - Removed the `parameters` argument from `_process_wedge_backward_complex`, `_apply_backward_transform_complex`, `_process_wedge_complex`, and `_apply_forward_transform_complex` functions, streamlining the function signatures. - Updated documentation to reflect the removal of the `parameters` argument, enhancing clarity and maintainability across the transformation implementations.
- Removed unnecessary type ignore comments in the `color_windows` and `plot_disk` functions within `plot_02_direction_resolution.py`, enhancing code clarity. - Streamlined the assignment of RGBA values to masks, improving overall code maintainability.
…ndling - Updated the return type of the `struct` method in the UDCT class to support both UDCTCoefficients and MUDCTCoefficients, enhancing type safety. - Removed redundant import of SparseWindow within the `struct` method, streamlining the code. - Improved handling of SparseWindow instances during window updates, ensuring consistency and clarity in the UDCT implementation.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
==========================================
- Coverage 94.26% 94.09% -0.17%
==========================================
Files 27 29 +2
Lines 1848 1881 +33
==========================================
+ Hits 1742 1770 +28
- Misses 106 111 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* refactor: migrate to PEP 735 dependency groups Migrate test/dev/docs from optional-dependencies to dependency-groups to fix uv's universal dependency resolution. This allows docs (sphinx>=8.2.1 requires Python 3.11+) to work while maintaining Python 3.9+ support. - Convert to [dependency-groups] with Python version markers for sphinx - Update nox/CI/ReadTheDocs to use `uv pip install --group` or `pip install --group` - Ensure docs/requirements.txt installs before groups (CPU-only torch) - Remove matplotlib extra, keep torch as optional Fixes uv resolution errors on Python 3.9/3.10 when installing docs extra. * fix(nox): install package in pylint session * fix(ci): install uv before running pylint session * fix(docs): install package in editable mode for sphinx autodoc * fix(readthedocs): install package for sphinx autodoc * fix(nox): make uv mandatory for venv backend * fix(nox): install package in tests session
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ax.yaxis.set_visible(False) | ||
| ax.grid(False) | ||
| ax.set_facecolor(color_bg) | ||
| ax.set_facecolor(str(color_bg) if isinstance(color_bg, tuple) else color_bg) |
There was a problem hiding this comment.
Color tuple conversion breaks matplotlib compatibility
Medium Severity
The plot_disk function converts tuple colors to strings using str(color_bg), which produces an invalid matplotlib color. When color_bg is the default tuple (0, 0, 0, 1), str() produces "(0, 0, 0, 1)" which matplotlib cannot parse as a color. The previous code passed tuples directly to ax.set_facecolor(), which handles them correctly. While the current example happens to pass a string for color_bg, the function's default value and type signature both allow tuples, so calling plot_disk(C, ax) without arguments would fail.
| else: | ||
| mask_target[..., 0][mask_index] = rgb_a[0] | ||
| mask_target[..., 1][mask_index] = rgb_a[1] | ||
| mask_target[..., 2][mask_index] = rgb_a[2] |
There was a problem hiding this comment.
Removed length check causes IndexError for RGB tuples
Low Severity
The refactored assign_rgba_to_mask function now always accesses rgb_a[3], but the old code checked if len(rgb_a) > 3 before accessing the alpha channel. The function signatures for color_windows and plot_disk accept tuple[int, ...] which includes 3-element RGB tuples. If a user passes a 3-tuple like (255, 0, 0) for color_bg or color_low, the code will raise an IndexError. The current example code happens to use strings, but the documented API contract is broken.
Additional Locations (1)
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
1 participant
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.
Note
Introduces a dedicated
SparseWindowabstraction and migrates the UDCT implementation to it for both NumPy and PyTorch.SparseWindow(methods:to_dense,multiply_extract,multiply_at_indices,scatter_add); updateUDCTWindowstyping and remove_from_sparse/from_sparse_newSparseWindowAPIs; simplify accumulation and window application; adjust window normalization logicUDCTWindow) to emitSparseWindow; refine Kronecker product construction and ensure contiguous outputsSparseWindow.to_dense()and type hints; minor plotting fixescurvelets.torch._sparse_window.SparseWindowand apply across transforms/windows; support dtype/device moves viaapply_to_tensors)_from_sparsetests, verifySparseWindowstructure, indices/values parity, and dtype conversions)Written by Cursor Bugbot for commit 630d596. This will update automatically on new commits. Configure here.