Refactor into mixins #14
Conversation
WalkthroughRefactors polymath extensions from module-level functions to mixin classes and updates Qube to inherit these mixins. Removes the centralized extension wiring in polymath/extensions/init.py and localizes imports; adds a flake8 per-file ignore for extension files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14 +/- ##
==========================================
- Coverage 72.83% 72.54% -0.29%
==========================================
Files 24 23 -1
Lines 7123 7074 -49
Branches 1599 1599
==========================================
- Hits 5188 5132 -56
- Misses 1497 1504 +7
Partials 438 438 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
polymath/extensions/pickler.py (1)
2-2: Fix file header comment to match filename.The comment says
pickle.pybut the file ispickler.py.-# polymath/extensions/pickle.py +# polymath/extensions/pickler.py
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
.flake8(1 hunks)polymath/__init__.py(0 hunks)polymath/extensions/__init__.py(1 hunks)polymath/extensions/broadcaster.py(1 hunks)polymath/extensions/indexer.py(1 hunks)polymath/extensions/item_ops.py(1 hunks)polymath/extensions/iterator.py(3 hunks)polymath/extensions/mask_ops.py(2 hunks)polymath/extensions/pickler.py(3 hunks)polymath/extensions/shaper.py(1 hunks)polymath/extensions/shrinker.py(1 hunks)polymath/extensions/tvl.py(1 hunks)polymath/extensions/vector_ops.py(1 hunks)polymath/qube.py(1 hunks)
💤 Files with no reviewable changes (1)
- polymath/init.py
🧰 Additional context used
🧬 Code graph analysis (5)
polymath/extensions/tvl.py (2)
polymath/qube.py (9)
Qube(23-3158)is_one_false(2239-2245)antimask(1304-1317)and_(930-967)prefer_builtins(337-352)as_builtin(354-388)mask(1298-1301)_set_mask(1208-1252)or_(890-927)polymath/extensions/math_ops.py (3)
logical_not(1615-1625)any(1631-1675)all(1677-1718)
polymath/extensions/broadcaster.py (2)
polymath/qube.py (6)
shape(1344-1347)Qube(23-3158)derivs(1338-1341)insert_deriv(1520-1586)item(1380-1383)_is_one_value(2248-2251)polymath/extensions/shaper.py (1)
reshape(11-50)
polymath/extensions/mask_ops.py (1)
polymath/qube.py (9)
mask(1298-1301)remask(2723-2753)Qube(23-3158)_suitable_mask(529-586)copy(2161-2208)remask_or(2755-2786)or_(890-927)derivs(1338-1341)shape(1344-1347)
polymath/extensions/shrinker.py (3)
polymath/qube.py (5)
antimask(1304-1317)Qube(23-3158)shape(1344-1347)mask(1298-1301)item(1380-1383)polymath/extensions/mask_ops.py (1)
mask_where(11-84)polymath/extensions/broadcaster.py (1)
broadcast_to(34-120)
polymath/extensions/vector_ops.py (3)
polymath/qube.py (11)
Qube(23-3158)antimask(1304-1317)shape(1344-1347)denom(1392-1395)ndim(1356-1359)or_(890-927)nrank(1368-1371)drank(1374-1377)wod(1725-1750)_disallow_denom(3010-3018)dtype(2257-2260)polymath/extensions/math_ops.py (4)
mean(1894-1926)sum(1860-1892)any(1631-1675)all(1677-1718)polymath/vector.py (1)
unit(443-456)
🪛 Ruff (0.14.7)
polymath/extensions/tvl.py
363-363: Missing return type annotation for private function _tvl_op
(ANN202)
polymath/extensions/broadcaster.py
123-123: Missing return type annotation for staticmethod broadcasted_shape
(ANN205)
123-123: Missing type annotation for *objects
(ANN002)
182-182: Avoid specifying long messages outside the exception class
(TRY003)
187-187: Missing return type annotation for staticmethod broadcast
(ANN205)
187-187: Missing type annotation for *objects
(ANN002)
polymath/extensions/indexer.py
12-12: Missing return type annotation for special method __getitem__
(ANN204)
45-45: Avoid specifying long messages outside the exception class
(TRY003)
96-96: Missing return type annotation for special method __setitem__
Add return type annotation: None
(ANN204)
104-104: Unpacked variable shape_before is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
244-244: Missing return type annotation for private function _prep_index
(ANN202)
314-314: Abstract raise to an inner function
(TRY301)
314-314: Avoid specifying long messages outside the exception class
(TRY003)
329-329: Abstract raise to an inner function
(TRY301)
329-329: Avoid specifying long messages outside the exception class
(TRY003)
356-356: Abstract raise to an inner function
(TRY301)
356-356: Avoid specifying long messages outside the exception class
(TRY003)
375-379: Abstract raise to an inner function
(TRY301)
375-379: Avoid specifying long messages outside the exception class
(TRY003)
494-494: Abstract raise to an inner function
(TRY301)
519-519: Do not catch blind exception: Exception
(BLE001)
520-520: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
522-522: Missing return type annotation for private function _prep_scalar_index
(ANN202)
560-560: Avoid specifying long messages outside the exception class
(TRY003)
562-562: Avoid specifying long messages outside the exception class
(TRY003)
572-572: Avoid specifying long messages outside the exception class
(TRY003)
580-580: Avoid specifying long messages outside the exception class
(TRY003)
588-588: Avoid specifying long messages outside the exception class
(TRY003)
polymath/extensions/iterator.py
117-117: Missing return type annotation for special method __iter__
(ANN204)
polymath/extensions/mask_ops.py
50-51: Avoid specifying long messages outside the exception class
(TRY003)
61-61: Boolean positional value in function call
(FBT003)
423-423: Missing return type annotation for private function _limit_from_qube
(ANN202)
483-483: Missing return type annotation for staticmethod is_below
(ANN205)
483-483: Boolean default positional argument in function definition
(FBT002)
502-502: Missing return type annotation for staticmethod is_above
(ANN205)
502-502: Boolean default positional argument in function definition
(FBT002)
521-521: Missing return type annotation for staticmethod is_outside
(ANN205)
521-521: Boolean default positional argument in function definition
(FBT002)
541-541: Missing return type annotation for staticmethod is_inside
(ANN205)
541-541: Boolean default positional argument in function definition
(FBT002)
polymath/extensions/shaper.py
138-138: Avoid specifying long messages outside the exception class
(TRY003)
196-196: Avoid specifying long messages outside the exception class
(TRY003)
237-237: Missing return type annotation for staticmethod stack
(ANN205)
237-237: Missing type annotation for *args
(ANN002)
287-288: Avoid specifying long messages outside the exception class
(TRY003)
340-340: Consider (len(args), *args[subclass_indx].shape) instead of concatenation
Replace with (len(args), *args[subclass_indx].shape)
(RUF005)
357-357: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
polymath/extensions/pickler.py
98-98: Missing return type annotation for staticmethod _pickle_debug
Add return type annotation: None
(ANN205)
161-161: Missing return type annotation for staticmethod set_default_pickle_digits
Add return type annotation: None
(ANN205)
237-237: Missing return type annotation for private function _check_pickle_digits
Add return type annotation: None
(ANN202)
255-255: Loop control variable key not used within loop body
Rename unused key to _key
(B007)
262-262: Missing return type annotation for staticmethod _validate_pickle_digits
(ANN205)
283-283: Abstract raise to an inner function
(TRY301)
288-288: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
293-293: Missing return type annotation for staticmethod _validate_pickle_reference
(ANN205)
314-314: Abstract raise to an inner function
(TRY301)
314-314: Avoid specifying long messages outside the exception class
(TRY003)
317-317: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
317-317: Avoid specifying long messages outside the exception class
(TRY003)
326-326: Missing return type annotation for staticmethod fpzip_compress
(ANN205)
358-358: Consider (-1, *shape[-3:]) instead of concatenation
Replace with (-1, *shape[-3:])
(RUF005)
394-394: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
400-400: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
401-401: Consider (-1, *shape[2:]) instead of concatenation
(RUF005)
412-412: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
415-415: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
421-421: Missing return type annotation for staticmethod fpzip_decompress
(ANN205)
476-476: Missing return type annotation for staticmethod _encode_one_float_array
(ANN205)
527-527: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
576-576: Missing return type annotation for staticmethod _encode_floats
(ANN205)
632-632: Consider (-1, item_size) instead of concatenation
Replace with (-1, item_size)
(RUF005)
643-643: Missing return type annotation for staticmethod _decode_scaled_uints
(ANN205)
668-668: Missing return type annotation for staticmethod _decode_floats
(ANN205)
691-691: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
699-699: Consider (len(items), *shape[:-item_rank]) instead of concatenation
Replace with (len(items), *shape[:-item_rank])
(RUF005)
707-707: Missing return type annotation for staticmethod _encode_ints
(ANN205)
716-716: Missing return type annotation for staticmethod _decode_ints
(ANN205)
723-723: Missing return type annotation for staticmethod _encode_bools
(ANN205)
732-732: Missing return type annotation for staticmethod _decode_bools
(ANN205)
745-745: Missing return type annotation for special method __getstate__
(ANN204)
886-886: Missing return type annotation for special method __setstate__
(ANN204)
966-966: Avoid specifying long messages outside the exception class
(TRY003)
polymath/extensions/item_ops.py
88-88: Consider (type(self), *classes) instead of concatenation
Replace with (type(self), *classes)
(RUF005)
111-111: Avoid specifying long messages outside the exception class
(TRY003)
217-217: Boolean default positional argument in function definition
(FBT002)
241-241: Avoid specifying long messages outside the exception class
(TRY003)
255-255: Boolean positional value in function call
(FBT003)
337-338: Avoid specifying long messages outside the exception class
(TRY003)
430-430: Loop control variable r not used within loop body
Rename unused r to _r
(B007)
459-459: Missing return type annotation for special method __matmul__
(ANN204)
polymath/extensions/vector_ops.py
13-13: Missing return type annotation for private function _mean_or_sum
(ANN202)
54-54: Boolean positional value in function call
(FBT003)
58-58: Boolean positional value in function call
(FBT003)
63-63: Boolean positional value in function call
(FBT003)
108-108: Missing return type annotation for private function _check_axis
Add return type annotation: None
(ANN202)
138-139: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
138-139: Avoid specifying long messages outside the exception class
(TRY003)
142-142: Avoid specifying long messages outside the exception class
(TRY003)
146-146: Missing return type annotation for private function _zero_sized_result
(ANN202)
170-170: Missing return type annotation for staticmethod dot
(ANN205)
207-208: Avoid specifying long messages outside the exception class
(TRY003)
217-218: Avoid specifying long messages outside the exception class
(TRY003)
223-224: Avoid specifying long messages outside the exception class
(TRY003)
283-283: Missing return type annotation for staticmethod norm
(ANN205)
314-315: Avoid specifying long messages outside the exception class
(TRY003)
339-339: Missing return type annotation for staticmethod norm_sq
(ANN205)
369-370: Avoid specifying long messages outside the exception class
(TRY003)
394-394: Missing return type annotation for staticmethod cross
(ANN205)
430-431: Avoid specifying long messages outside the exception class
(TRY003)
440-441: Avoid specifying long messages outside the exception class
(TRY003)
446-447: Avoid specifying long messages outside the exception class
(TRY003)
510-510: Missing return type annotation for staticmethod cross_3x3
(ANN205)
530-530: Avoid specifying long messages outside the exception class
(TRY003)
540-540: Missing return type annotation for staticmethod cross_2x2
(ANN205)
560-560: Avoid specifying long messages outside the exception class
(TRY003)
566-566: Missing return type annotation for staticmethod outer
(ANN205)
566-566: Boolean default positional argument in function definition
(FBT002)
642-642: Missing return type annotation for staticmethod as_diagonal
(ANN205)
642-642: Boolean default positional argument in function definition
(FBT002)
668-669: Avoid specifying long messages outside the exception class
(TRY003)
693-693: Boolean positional value in function call
(FBT003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test polymath (windows-latest, 3.12)
- GitHub Check: Test polymath (windows-latest, 3.8)
- GitHub Check: Test polymath (windows-latest, 3.10)
- GitHub Check: Test polymath (macos-latest, 3.8)
🔇 Additional comments (29)
polymath/extensions/shrinker.py (1)
106-188: LGTM!The
unshrink()method correctly handles cached values, shape reconstruction, value/mask expansion, and derivative propagation.polymath/extensions/item_ops.py (1)
438-473: LGTM!The
chain()and__matmul__()methods are implemented correctly with proper delegation.polymath/extensions/pickler.py (2)
745-884: LGTM!The
__getstate__method comprehensively handles mask/value encoding, derivative serialization, and various data types with appropriate compression strategies.
886-1016: LGTM!The
__setstate__method correctly reverses the serialization process, handling backward compatibility for renamed keys, proper mask/value decoding, and derivative restoration with antimask expansion.polymath/extensions/iterator.py (3)
9-62: LGTM!The private
_QubeIteratorclass correctly implements first-axis iteration with proper__iter__and__next__methods.
64-113: LGTM!The private
_QubeNDIteratorclass correctly implements multi-dimensional iteration usingitertools.product.
116-137: LGTM!The public
QubeIteratormixin cleanly delegates to private iterator implementations, providing a clean public API.polymath/extensions/indexer.py (1)
8-10: LGTM! Mixin class structure is appropriate.The
QubeIndexermixin encapsulates indexing operations cleanly and follows the pattern used by other mixins in this PR.polymath/extensions/broadcaster.py (1)
8-9: LGTM! Mixin structure is appropriate.The
QubeBroadcastermixin cleanly encapsulates broadcast operations with appropriate docstrings for the deprecatedbroadcast_into_shapemethod.polymath/qube.py (1)
10-25: LGTM! Mixin composition is well-structured.The multi-inheritance approach cleanly separates concerns into dedicated mixins. The import organization and class declaration follow good practices for Python mixin patterns.
polymath/extensions/shaper.py (1)
328-336: LGTM! Mask determination logic is correct.The handling of
Noneplaceholders and mask type determination is properly implemented.polymath/extensions/tvl.py (2)
36-73: LGTM! Three-valued logic implementation is correct.The
tvl_andimplementation correctly follows the documented truth table for three-valued logic AND operation. The handling of masked values as indeterminate is properly implemented.
397-400: The mutation is safe and not a concern.Comparison operators (
==,<, etc.) always return freshly instantiated Boolean objects via theQube._BOOLEAN_CLASS(...)constructor, not cached or reused objects. Since_set_mask()is called on a newly created object with no other references, the in-place mutation has no side effects. This pattern is consistent throughout the codebase.polymath/extensions/mask_ops.py (7)
8-9: LGTM - Clean mixin class definition.The mixin pattern is appropriate for extending Qube functionality.
11-84: LGTM - Well-structured mask_where implementation.The method correctly handles all cases: scalar, no replacement, and with replacement. The early return optimization at line 36-37 is good.
86-137: LGTM - Correct implementation of equality-based masking.The use of
np.allacross item axes correctly handles multi-dimensional items.
139-246: LGTM - Consistent comparison-based masking methods.The use of ±infinity for masked limit values correctly ensures masked positions don't trigger masking.
248-354: LGTM - Range-based masking methods implemented correctly.The operator selection logic based on
mask_endpointsis correct, and the masked limit values are appropriately chosen to make masked limits effectively ignored.
356-421: LGTM - Comprehensive clip implementation.The method correctly handles both the optimized scalar case and the general case. The derivative handling when
remask=Falseproperly zeroes out derivatives for clipped values.
482-558: LGTM - Clean utility methods for range checking.These static methods provide clear, reusable range-checking logic. The static analysis hints about return type annotations are valid but optional improvements.
polymath/extensions/vector_ops.py (9)
10-11: LGTM - Appropriate mixin class for vector operations.The class consolidates vector operations that will be mixed into the Qube class.
13-106: LGTM - Comprehensive mean/sum implementation.The method correctly handles all masking scenarios and properly propagates derivatives. The axis normalization logic is correct.
169-279: LGTM - Correct dot product implementation.The axis handling, shape manipulation, and derivative propagation logic is correct.
283-390: LGTM - Correct norm and norm_sq implementations.The derivative calculations correctly apply the chain rule.
394-507: LGTM - Correct cross product implementation.The method properly handles both 2D and 3D cross products with appropriate result shapes.
509-562: LGTM - Correct cross product helper implementations.The standard cross product formulas are correctly implemented.
566-638: LGTM - Correct outer product implementation.The shape manipulation for outer product and derivative propagation is correct.
642-695: LGTM - Correct as_diagonal implementation.The method correctly expands a single axis into a diagonal matrix representation.
697-711: LGTM - Simple and correct RMS implementation.The method correctly computes root-mean-square across item dimensions.
|
This work is postponed due to unforeseen problems with Sphinx. See #15. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (10)
polymath/extensions/shrinker.py (1)
47-50: Inconsistent read-only handling for shapeless objects remains unaddressed.The docstring at line 23 states "Shrunken objects are always converted to read-only," but when
self._is_scalar, the method returnsselfwithout callingas_readonly(). Other code paths (lines 42, 86, 97) consistently callas_readonly().# If this is a shapeless object, return it as is if self._is_scalar: - self._cache['unshrunk'] = self - return self + obj = self.as_readonly() + obj._cache['unshrunk'] = self + return objpolymath/extensions/indexer.py (1)
103-104: Prefix unused variable with underscore.
shape_beforeis unpacked but never used. Use_shape_beforeto signal intent and silence linter warnings.try: (masked, size_zero, - shape_before, shape_after) = self._prep_scalar_index(indx) + _shape_before, shape_after) = self._prep_scalar_index(indx)polymath/extensions/shaper.py (2)
149-152: Self-reassignment pattern is intentional but could use clarification.The reassignment of
selfcreates a reshaped view for subsequent operations. As noted in a previous review, a brief comment would clarify this intentional pattern.# Add missing axes if necessary if self._ndims < rank: + # Reassign self to a reshaped view with leading singleton axes self = self.reshape((rank - self._ndims) * (1,) + self._shape, recursive=recursive)
212-215: Same self-reassignment pattern asroll_axis.Consider adding a similar clarifying comment here for consistency.
# Add missing axes if necessary if self._ndims < rank: + # Reassign self to a reshaped view with leading singleton axes self = self.reshape((rank - self._ndims) * (1,) + self._shape, recursive=recursive)polymath/extensions/item_ops.py (3)
8-9: Class docstring could be more descriptive.The docstring is generic. Consider specifying that this mixin provides numerator/denominator item restructuring operations.
56-91: Use tuple unpacking for cleaner syntax.Static analysis suggests using tuple unpacking instead of concatenation.
- obj = obj.cast((type(self),) + classes) + obj = obj.cast((type(self), *classes))
411-436: Rename unused loop variable to_r.The loop variable
ris not used within the loop body.- for r in range(self._nrank): + for _r in range(self._nrank): new_values = np.rollaxis(new_values, -self._drank-1, len_shape)polymath/extensions/vector_ops.py (1)
160-169: Bug: Misplaced else clause causes incorrect behavior.The
elseon line 164 is a for-else construct (executes after loop completion), not an if-else. This means line 165indx[i] = 0runs after the loop withiset to the last iterated value, which is redundant. Additionally, whenaxisis a single integer (not a list/tuple), the code falls through to line 167 correctly, but the for-else is confusing and incorrect.# Construct an index to obtain the correct shape indx = self._ndims * [slice(None)] if isinstance(axis, (list, tuple)): for i in axis: indx[i] = 0 - else: - indx[i] = 0 + else: + indx[axis] = 0 return self[tuple(indx)]polymath/extensions/pickler.py (2)
287-288: Use exception chaining for clearer tracebacks.When re-raising exceptions inside an
exceptblock, useraise ... from Noneto suppress the original exception context when it's not relevant.
316-317: Use exception chaining for clearer tracebacks.When re-raising exceptions inside an
exceptblock, useraise ... from None.except (ValueError, IndexError, TypeError): - raise ValueError(f'invalid pickle reference {original_references!r}') + raise ValueError(f'invalid pickle reference {original_references!r}') from None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
polymath/extensions/__init__.py(1 hunks)polymath/extensions/indexer.py(1 hunks)polymath/extensions/item_ops.py(1 hunks)polymath/extensions/mask_ops.py(2 hunks)polymath/extensions/pickler.py(3 hunks)polymath/extensions/shaper.py(1 hunks)polymath/extensions/shrinker.py(1 hunks)polymath/extensions/tvl.py(1 hunks)polymath/extensions/vector_ops.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
polymath/extensions/shaper.py (2)
polymath/qube.py (19)
shape(1344-1347)Qube(23-3158)insert_deriv(1520-1586)rank(1362-1365)denom(1392-1395)is_float(2291-2296)is_int(2344-2351)is_bool(2395-2400)confirm_unit(1949-1966)drank(1374-1377)as_this_type(2439-2516)is_one_true(2230-2236)is_one_false(2239-2245)mask(1298-1301)zeros(1030-1051)dtype(2257-2260)values(1259-1262)derivs(1338-1341)insert_derivs(1588-1620)polymath/extensions/broadcaster.py (1)
broadcast(187-229)
polymath/extensions/indexer.py (3)
polymath/qube.py (12)
as_size_zero(2585-2624)shape(1344-1347)copy(2161-2208)dtype(2257-2260)mask(1298-1301)Qube(23-3158)insert_deriv(1520-1586)antimask(1304-1317)denom(1392-1395)item(1380-1383)_disallow_denom(3010-3018)is_float(2291-2296)polymath/scalar.py (1)
as_index_and_mask(142-199)polymath/vector.py (1)
as_index_and_mask(200-254)
polymath/extensions/mask_ops.py (1)
polymath/qube.py (8)
mask(1298-1301)remask(2723-2753)Qube(23-3158)_suitable_mask(529-586)copy(2161-2208)derivs(1338-1341)_opstr(2990-3008)shape(1344-1347)
polymath/extensions/item_ops.py (1)
polymath/qube.py (11)
Qube(23-3158)_require_axis_in_range(3030-3045)nrank(1368-1371)cast(2518-2554)insert_deriv(1520-1586)drank(1374-1377)shape(1344-1347)nsize(1410-1413)_opstr(2990-3008)dsize(1416-1419)wod(1725-1750)
polymath/extensions/pickler.py (1)
polymath/qube.py (5)
values(1259-1262)dtype(2257-2260)shape(1344-1347)Qube(23-3158)_dtype(673-676)
polymath/extensions/vector_ops.py (2)
polymath/qube.py (6)
Qube(23-3158)denom(1392-1395)or_(890-927)drank(1374-1377)_disallow_denom(3010-3018)dtype(2257-2260)polymath/extensions/math_ops.py (4)
mean(1894-1926)sum(1860-1892)any(1631-1675)all(1677-1718)
🪛 Ruff (0.14.7)
polymath/extensions/shaper.py
138-138: Avoid specifying long messages outside the exception class
(TRY003)
196-196: Avoid specifying long messages outside the exception class
(TRY003)
236-236: Missing return type annotation for staticmethod stack
(ANN205)
236-236: Missing type annotation for *args
(ANN002)
286-287: Avoid specifying long messages outside the exception class
(TRY003)
339-339: Consider (len(args), *args[subclass_indx].shape) instead of concatenation
Replace with (len(args), *args[subclass_indx].shape)
(RUF005)
356-356: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
polymath/extensions/indexer.py
12-12: Missing return type annotation for special method __getitem__
(ANN204)
45-45: Avoid specifying long messages outside the exception class
(TRY003)
96-96: Missing return type annotation for special method __setitem__
Add return type annotation: None
(ANN204)
104-104: Unpacked variable shape_before is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
244-244: Missing return type annotation for private function _prep_index
(ANN202)
314-314: Abstract raise to an inner function
(TRY301)
314-314: Avoid specifying long messages outside the exception class
(TRY003)
329-329: Abstract raise to an inner function
(TRY301)
329-329: Avoid specifying long messages outside the exception class
(TRY003)
356-356: Abstract raise to an inner function
(TRY301)
356-356: Avoid specifying long messages outside the exception class
(TRY003)
375-379: Abstract raise to an inner function
(TRY301)
375-379: Avoid specifying long messages outside the exception class
(TRY003)
494-494: Abstract raise to an inner function
(TRY301)
522-522: Missing return type annotation for private function _prep_scalar_index
(ANN202)
560-560: Avoid specifying long messages outside the exception class
(TRY003)
562-562: Avoid specifying long messages outside the exception class
(TRY003)
572-572: Avoid specifying long messages outside the exception class
(TRY003)
580-580: Avoid specifying long messages outside the exception class
(TRY003)
588-588: Avoid specifying long messages outside the exception class
(TRY003)
polymath/extensions/mask_ops.py
50-51: Avoid specifying long messages outside the exception class
(TRY003)
61-61: Boolean positional value in function call
(FBT003)
423-423: Missing return type annotation for private function _limit_from_qube
(ANN202)
483-483: Missing return type annotation for staticmethod is_below
(ANN205)
483-483: Boolean default positional argument in function definition
(FBT002)
502-502: Missing return type annotation for staticmethod is_above
(ANN205)
502-502: Boolean default positional argument in function definition
(FBT002)
521-521: Missing return type annotation for staticmethod is_outside
(ANN205)
521-521: Boolean default positional argument in function definition
(FBT002)
541-541: Missing return type annotation for staticmethod is_inside
(ANN205)
541-541: Boolean default positional argument in function definition
(FBT002)
polymath/extensions/tvl.py
362-362: Missing return type annotation for private function _tvl_op
(ANN202)
polymath/extensions/item_ops.py
88-88: Consider (type(self), *classes) instead of concatenation
Replace with (type(self), *classes)
(RUF005)
111-111: Avoid specifying long messages outside the exception class
(TRY003)
217-217: Boolean default positional argument in function definition
(FBT002)
241-241: Avoid specifying long messages outside the exception class
(TRY003)
255-255: Boolean positional value in function call
(FBT003)
337-338: Avoid specifying long messages outside the exception class
(TRY003)
430-430: Loop control variable r not used within loop body
Rename unused r to _r
(B007)
459-459: Missing return type annotation for special method __matmul__
(ANN204)
polymath/extensions/pickler.py
98-98: Missing return type annotation for staticmethod _pickle_debug
Add return type annotation: None
(ANN205)
161-161: Missing return type annotation for staticmethod set_default_pickle_digits
Add return type annotation: None
(ANN205)
237-237: Missing return type annotation for private function _check_pickle_digits
Add return type annotation: None
(ANN202)
255-255: Loop control variable key not used within loop body
Rename unused key to _key
(B007)
262-262: Missing return type annotation for staticmethod _validate_pickle_digits
(ANN205)
283-283: Abstract raise to an inner function
(TRY301)
293-293: Missing return type annotation for staticmethod _validate_pickle_reference
(ANN205)
296-296: Unused blanket noqa directive
Remove unused noqa directive
(RUF100)
314-314: Abstract raise to an inner function
(TRY301)
314-314: Avoid specifying long messages outside the exception class
(TRY003)
317-317: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
317-317: Avoid specifying long messages outside the exception class
(TRY003)
326-326: Missing return type annotation for staticmethod fpzip_compress
(ANN205)
358-358: Consider (-1, *shape[-3:]) instead of concatenation
Replace with (-1, *shape[-3:])
(RUF005)
394-394: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
400-400: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
401-401: Consider (-1, *shape[2:]) instead of concatenation
(RUF005)
423-423: Missing return type annotation for staticmethod fpzip_decompress
(ANN205)
478-478: Missing return type annotation for staticmethod _encode_one_float_array
(ANN205)
529-529: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
578-578: Missing return type annotation for staticmethod _encode_floats
(ANN205)
634-634: Consider (-1, item_size) instead of concatenation
Replace with (-1, item_size)
(RUF005)
645-645: Missing return type annotation for staticmethod _decode_scaled_uints
(ANN205)
670-670: Missing return type annotation for staticmethod _decode_floats
(ANN205)
693-693: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
701-701: Consider (len(items), *shape[:-item_rank]) instead of concatenation
Replace with (len(items), *shape[:-item_rank])
(RUF005)
709-709: Missing return type annotation for staticmethod _encode_ints
(ANN205)
718-718: Missing return type annotation for staticmethod _decode_ints
(ANN205)
725-725: Missing return type annotation for staticmethod _encode_bools
(ANN205)
734-734: Missing return type annotation for staticmethod _decode_bools
(ANN205)
747-747: Missing return type annotation for special method __getstate__
(ANN204)
888-888: Missing return type annotation for special method __setstate__
(ANN204)
968-968: Avoid specifying long messages outside the exception class
(TRY003)
polymath/extensions/vector_ops.py
13-13: Missing return type annotation for private function _mean_or_sum
(ANN202)
54-54: Boolean positional value in function call
(FBT003)
58-58: Boolean positional value in function call
(FBT003)
63-63: Boolean positional value in function call
(FBT003)
108-108: Missing return type annotation for private function _check_axis
Add return type annotation: None
(ANN202)
138-139: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
138-139: Avoid specifying long messages outside the exception class
(TRY003)
142-142: Avoid specifying long messages outside the exception class
(TRY003)
146-146: Missing return type annotation for private function _zero_sized_result
(ANN202)
172-172: Missing return type annotation for staticmethod dot
(ANN205)
209-210: Avoid specifying long messages outside the exception class
(TRY003)
219-220: Avoid specifying long messages outside the exception class
(TRY003)
225-226: Avoid specifying long messages outside the exception class
(TRY003)
284-284: Missing return type annotation for staticmethod norm
(ANN205)
315-316: Avoid specifying long messages outside the exception class
(TRY003)
339-339: Missing return type annotation for staticmethod norm_sq
(ANN205)
369-370: Avoid specifying long messages outside the exception class
(TRY003)
393-393: Missing return type annotation for staticmethod cross
(ANN205)
429-430: Avoid specifying long messages outside the exception class
(TRY003)
439-440: Avoid specifying long messages outside the exception class
(TRY003)
445-446: Avoid specifying long messages outside the exception class
(TRY003)
509-509: Missing return type annotation for staticmethod cross_3x3
(ANN205)
529-529: Avoid specifying long messages outside the exception class
(TRY003)
539-539: Missing return type annotation for staticmethod cross_2x2
(ANN205)
559-559: Avoid specifying long messages outside the exception class
(TRY003)
564-564: Missing return type annotation for staticmethod outer
(ANN205)
564-564: Boolean default positional argument in function definition
(FBT002)
639-639: Missing return type annotation for staticmethod as_diagonal
(ANN205)
639-639: Boolean default positional argument in function definition
(FBT002)
665-666: Avoid specifying long messages outside the exception class
(TRY003)
690-690: Boolean positional value in function call
(FBT003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test polymath (macos-latest, 3.10)
- GitHub Check: Test polymath (windows-latest, 3.8)
🔇 Additional comments (47)
polymath/extensions/__init__.py (1)
4-4: LGTM!The comment correctly indicates that mixins are now imported directly by
qube.py. The typo from the previous review ("directory" → "directly") has been addressed.polymath/extensions/shrinker.py (3)
8-9: Docstring improvement applied.The docstring now correctly describes "shrink and unshrink operations" as suggested in previous review.
94-98: Verifyas_readonly()return value is intentionally discarded.At line 97,
obj.as_readonly()is called but the return value is not captured. This works correctly only ifas_readonly()modifiesobjin-place and returnsself. Other code paths (lines 42, 86) assign the result. For consistency and safety, consider capturing the return value.# Construct the new object obj = Qube.__new__(type(self)) obj.__init__(self._values[extras * (slice(None),) + (antimask, Ellipsis)], mask, example=self) - obj.as_readonly() + obj = obj.as_readonly()
106-187: LGTM!The
unshrinkmethod is well-implemented with proper read-only handling (line 181), correct derivative propagation, and appropriate local imports to avoid circular dependencies.polymath/extensions/indexer.py (5)
9-10: LGTM!The
QubeIndexermixin class is well-structured with clear organization of indexing operations.
12-94: LGTM!The
__getitem__implementation correctly handles both shapeless and shaped indexing, with proper derivative propagation and readonly status preservation.
519-520: Exception chaining implemented correctly.The exception handling now properly chains the original exception with
from e, preserving the traceback context as suggested in the previous review.
244-521: LGTM!The
_prep_indexmethod is comprehensive, handling various index types (slices, ellipsis, boolean arrays, scalar arrays) with proper validation and error handling.
522-593: LGTM!The
_prep_scalar_indexmethod correctly handles all valid scalar index types (True, False, Ellipsis, None, empty slice) with clear error messages for invalid cases.polymath/extensions/shaper.py (2)
235-236: Duplicate@staticmethoddecorator removed.The duplicate decorator issue from the previous review has been addressed. Only one
@staticmethoddecorator is now present.
236-390: LGTM!The
stackmethod is comprehensive and handles type coercion, broadcasting, mask construction, and derivative propagation correctly. The implementation aligns with the pattern inpolymath/qube.py.polymath/extensions/tvl.py (3)
90-92: Stray docstring fragment removed.The duplicate/leftover docstring fragment noted in the previous review has been addressed. The docstring now reads cleanly.
362-399: LGTM!The
_tvl_ophelper correctly handles mask propagation for comparison operations. The in-place mask modification via_set_maskis safe since comparison operators (==,!=, etc.) return newly constructed objects.
11-250: LGTM!The three-valued logic methods (
tvl_and,tvl_or,tvl_any,tvl_all) correctly implement the truth tables documented in their docstrings, with proper handling of masked values as indeterminate.polymath/extensions/mask_ops.py (5)
8-9: LGTM!The
QubeMaskOpsmixin class correctly encapsulates all masking-related operations with a clear docstring.
11-84: LGTM!The
mask_wheremethod is well-implemented with proper handling of replacement values, shapeless objects, and derivative propagation through the indexing operation.
472-476: Inconsistent property access fixed.The code now consistently uses
limit._maskthroughout the method, addressing the previous review comment.
482-558: LGTM!The static helper methods (
is_below,is_above,is_outside,is_inside) are clean implementations of range-checking utilities that support theclipmethod.
356-421: LGTM!The
clipmethod correctly handles both scalar and Qube-based limits with proper derivative zeroing for clipped values whenremask=False.polymath/extensions/item_ops.py (9)
11-54: LGTM!The
extract_numermethod correctly handles axis positioning, array slicing viarollaxis, object construction with propernrankadjustment, and recursive derivative handling.
93-120: LGTM!The
extract_denomsmethod correctly handles edge cases (drank == 0 returns self, drank != 1 raises error) and properly constructs objects usingQube.__new__with correct initialization.
122-167: LGTM!The
slice_numermethod correctly handles slicing with proper axis positioning, rolls the axis back after slicing, and propagates derivatives recursively.
173-215: LGTM!The
transpose_numermethod correctly validates both axes, handles negative indices, swaps axes usingnp.swapaxes, and properly propagates derivatives.
259-274: LGTM!The
flatten_numermethod is a clean delegation toreshape_numer.
280-316: LGTM!The
transpose_denommethod correctly validates axes, handles negative indices, and properly constructs the result object. No recursive derivative handling is needed for denominator operations.
318-355: LGTM!The
reshape_denomandflatten_denommethods correctly handle denominator reshaping with proper validation and object construction.
361-409: LGTM!The
join_itemsandsplit_itemsmethods correctly handle the transfer of axes between numerator and denominator, stripping derivatives as documented.
438-473: LGTM!The
chainand__matmul__methods correctly implement chain multiplication semantics by flattening, joining items, and delegating toQube.dot. The__matmul__properly delegates tochain.polymath/extensions/vector_ops.py (9)
13-106: LGTM!The
_mean_or_summethod correctly handles all edge cases: zero-size arrays, fully masked/unmasked arrays, partial masks with proper counting, and recursive derivative handling. The axis normalization and NumPy function selection are correct.
171-281: LGTM!The
dotstatic method correctly handles:
- Dual denominator validation
- Axis positioning and validation for both operands
- Shape broadcasting for combined numerator/denominator structures
- Proper derivative propagation from both operands
283-336: LGTM!The
normstatic method correctly computes the Euclidean norm along the specified axis, validates the axis range, disallows denominators, and properly propagates derivatives using the chain rule (d/dx ||v|| = v/||v|| · dv/dx).
338-390: LGTM!The
norm_sqstatic method correctly computes the squared norm, handles units properly viaUnit.mul_units, and propagates derivatives using the chain rule (d/dx ||v||² = 2v · dv/dx).
392-506: LGTM!The
crossstatic method correctly:
- Validates axis lengths (must be 2 or 3)
- Handles both 2D and 3D cross products
- Properly manages numerator ranks for 2D (reduces by 2) vs 3D (reduces by 1)
- Propagates derivatives from both operands
508-561: LGTM!The
cross_3x3andcross_2x2static methods correctly implement the cross product formulas with proper broadcasting and shape validation.
563-636: LGTM!The
outerstatic method correctly computes the outer product by reshaping arrays to broadcast properly, validates dual denominators, and propagates derivatives from both operands.
638-692: LGTM!The
as_diagonalstatic method correctly converts an axis to a diagonal by creating a zero-filled array and populating the diagonal elements, with proper axis roll operations and recursive derivative handling.
694-709: LGTM!The
rmsmethod correctly computes the root-mean-square by summing squares across all item axes and dividing byisize.polymath/extensions/pickler.py (10)
102-159: LGTM!The
set_pickle_digitsmethod correctly validates inputs using the static validation methods, sets instance attributes, and propagates settings to derivatives.
160-206: LGTM!The
set_default_pickle_digitsstatic method correctly updates global defaults with proper validation.
207-259: LGTM!The
pickle_digits,pickle_reference, and_check_pickle_digitsmethods correctly handle lazy initialization with global defaults and propagate settings to derivatives.
325-420: LGTM!The
fpzip_compressmethod correctly handles:
- Precision calculation based on digits and dtype
- Dimension limiting to 4 axes (fpzip constraint)
- Retry logic for precision and buffer overflow errors
- Warning emission with proper stacklevel
422-471: LGTM!The
fpzip_decompressmethod correctly decompresses and applies bias correction using the alternating bit pattern to compensate for systematic truncation towards zero.
477-575: LGTM!The
_encode_one_float_arraymethod correctly handles multiple encoding strategies (fpzip, constant, scaled integers) with proper precision calculation and byte-efficient storage selection.
577-706: LGTM!The
_encode_floatsand_decode_floatsmethods correctly handle all encoding formats (literal, fpzip, constant, scaled, items) with proper shape handling and item-wise encoding for heterogeneous ranges.
708-741: LGTM!The integer and boolean encoding/decoding methods correctly use BZ2 compression with proper handling of contiguous arrays and packbits for booleans.
747-886: LGTM!The
__getstate__method correctly:
- Creates a shallow clone without recursive derivatives
- Handles scalar values, fully masked objects, and arrays
- Encodes masks with corner optimization
- Encodes values based on dtype (float/int/bool)
- Processes derivatives with antimask application
888-1017: LGTM!The
__setstate__method correctly:
- Handles legacy key renaming
- Decodes masks in reverse order (CORNERS, BOOL)
- Decodes values in reverse order (ALL_MASKED, ANTIMASKED, FLOAT, BOOL, INT)
- Restores readonly status and writability
- Expands derivatives with antimask restoration
| def reshape_numer(self, shape, classes=(), recursive=True): | ||
| """This object with a new shape for numerator items. | ||
| Raises: | ||
| ValueError: If the item size would be changed by the reshape operation. | ||
| """ | ||
| Parameters: | ||
| shape (tuple): The new shape for numerator items. | ||
| classes (class, list, or tuple, optional): The class of the object returned. If | ||
| a list is provided, the object will be an instance of the first suitable class | ||
| in the list. Otherwise, a generic Qube object will be returned. | ||
| recursive (bool, optional): True to reshape the derivatives in the same way; | ||
| otherwise, the returned object will not contain derivatives. | ||
| # Validate the shape | ||
| shape = tuple(shape) | ||
| if self.nsize != int(np.prod(shape)): | ||
| opstr = self._opstr('reshape_numer()') | ||
| raise ValueError(f'{opstr} item size must be unchanged: {self._numer}, {shape}') | ||
| Returns: | ||
| Qube: The reshaped object. | ||
| # Reshape | ||
| full_shape = self._shape + shape + self._denom | ||
| new_values = np.asarray(self._values).reshape(full_shape) | ||
| Raises: | ||
| ValueError: If the item size would be changed by the reshape operation. | ||
| """ | ||
|
|
||
| # Construct and cast | ||
| obj = Qube(new_values, self._mask, nrank=len(shape), example=self) | ||
| obj = obj.cast(classes) | ||
| obj._readonly = self._readonly | ||
| from polymath.qube import Qube | ||
|
|
||
| # Reshape the derivatives if necessary | ||
| if recursive: | ||
| for key, deriv in self._derivs.items(): | ||
| obj.insert_deriv(key, deriv.reshape_numer(shape, classes, False)) | ||
| # Validate the shape | ||
| shape = tuple(shape) | ||
| if self.nsize != int(np.prod(shape)): | ||
| opstr = self._opstr('reshape_numer()') | ||
| raise ValueError(f'{opstr} item size must be unchanged: {self._numer}, {shape}') | ||
|
|
||
| return obj | ||
| # Reshape | ||
| full_shape = self._shape + shape + self._denom | ||
| new_values = np.asarray(self._values).reshape(full_shape) | ||
|
|
||
| # Construct and cast | ||
| obj = Qube(new_values, self._mask, nrank=len(shape), example=self) | ||
| obj = obj.cast(classes) | ||
| obj._readonly = self._readonly | ||
|
|
||
| def flatten_numer(self, classes=(), *, recursive=True): | ||
| """This object with a new numerator shape such that nrank == 1. | ||
| # Reshape the derivatives if necessary | ||
| if recursive: | ||
| for key, deriv in self._derivs.items(): | ||
| obj.insert_deriv(key, deriv.reshape_numer(shape, classes, False)) | ||
|
|
||
| Parameters: | ||
| classes (class, list, or tuple, optional): The class of the object returned. If | ||
| a list is provided, the object will be an instance of the first suitable class | ||
| in the list. Otherwise, a generic Qube object will be returned. | ||
| recursive (bool, optional): True to include matching slices of the derivatives in | ||
| the returned object; otherwise, the returned object will not contain | ||
| derivatives. | ||
| return obj |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Boolean positional argument should be keyword-only.
The recursive parameter is a boolean positional argument which can lead to confusing calls like reshape_numer(shape, classes, True). Consider making it keyword-only for consistency with other methods.
- def reshape_numer(self, shape, classes=(), recursive=True):
+ def reshape_numer(self, shape, classes=(), *, recursive=True):Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.7)
217-217: Boolean default positional argument in function definition
(FBT002)
241-241: Avoid specifying long messages outside the exception class
(TRY003)
255-255: Boolean positional value in function call
(FBT003)
🤖 Prompt for AI Agents
In polymath/extensions/item_ops.py around lines 217 to 257, the boolean
positional parameter "recursive" should be made keyword-only to avoid confusing
calls; change the function signature to make recursive keyword-only (e.g., add a
bare * before recursive) and update the docstring if needed to keep parameter
docs aligned, then run a project-wide search for calls to reshape_numer and
update any call sites that currently pass recursive positionally (e.g.,
reshape_numer(shape, classes, True) -> reshape_numer(shape, classes,
recursive=True)).
| class QubePickler: | ||
| """Mix-in class providing operations for Qube objects.""" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Class docstring is too generic.
The docstring "Mix-in class providing operations for Qube objects" doesn't describe the pickling functionality. Consider "Mix-in class providing pickle serialization and compression operations for Qube objects."
🤖 Prompt for AI Agents
In polymath/extensions/pickler.py around lines 94 to 95, the class docstring for
QubePickler is too generic; update it to clearly state that this mix-in provides
pickle serialization and optional compression/decompression operations for Qube
objects (e.g., "Mix-in class providing pickle serialization and compression
operations for Qube objects."), ensuring it mentions serialization and
compression so future readers understand the class responsibilities.
| for key, deriv in self._derivs.items(): | ||
| if not hasattr(deriv, '_pickle_digits'): | ||
| deriv._pickle_digits = 2 * self._pickle_digits[1:] | ||
| if not hasattr(deriv, '_pickle_reference'): | ||
| deriv._pickle_reference = 2 * self._pickle_reference[1:] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Rename unused loop variable to _key.
The loop variable key is not used within the loop body.
- for key, deriv in self._derivs.items():
+ for _key, deriv in self._derivs.items():
if not hasattr(deriv, '_pickle_digits'):
deriv._pickle_digits = 2 * self._pickle_digits[1:]
if not hasattr(deriv, '_pickle_reference'):
deriv._pickle_reference = 2 * self._pickle_reference[1:]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for key, deriv in self._derivs.items(): | |
| if not hasattr(deriv, '_pickle_digits'): | |
| deriv._pickle_digits = 2 * self._pickle_digits[1:] | |
| if not hasattr(deriv, '_pickle_reference'): | |
| deriv._pickle_reference = 2 * self._pickle_reference[1:] | |
| for _key, deriv in self._derivs.items(): | |
| if not hasattr(deriv, '_pickle_digits'): | |
| deriv._pickle_digits = 2 * self._pickle_digits[1:] | |
| if not hasattr(deriv, '_pickle_reference'): | |
| deriv._pickle_reference = 2 * self._pickle_reference[1:] |
🧰 Tools
🪛 Ruff (0.14.7)
255-255: Loop control variable key not used within loop body
Rename unused key to _key
(B007)
🤖 Prompt for AI Agents
In polymath/extensions/pickler.py around lines 255 to 259, the loop variable
"key" is unused and should be renamed to "_key" to satisfy linters and make
intent clear; change the loop header from "for key, deriv in
self._derivs.items():" to "for _key, deriv in self._derivs.items():" (leave the
rest of the loop body unchanged).
| class QubeShaper: | ||
| """Mix-in class providing operations for Qube objects.""" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a more specific docstring.
The docstring "Mix-in class providing operations for Qube objects" is generic. Consider describing the reshaping/axis manipulation functionality specifically, similar to how QubeShrinker was updated.
class QubeShaper:
- """Mix-in class providing operations for Qube objects."""
+ """Mix-in class providing reshape and axis manipulation operations for Qube objects."""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class QubeShaper: | |
| """Mix-in class providing operations for Qube objects.""" | |
| class QubeShaper: | |
| """Mix-in class providing reshape and axis manipulation operations for Qube objects.""" |
🤖 Prompt for AI Agents
In polymath/extensions/shaper.py around lines 8 to 9, the class docstring is too
generic; replace it with a specific description of the reshaping and
axis-manipulation responsibilities of QubeShaper (e.g., explain it provides
methods to reshape, transpose, insert/remove/reorder axes, and preserve
coordinate metadata), matching the level of detail used in QubeShrinker so
readers and docs understand its exact purpose and behavior.
| class QubeTVL: | ||
| """Mix-in class providing operations for Qube objects.""" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a more specific docstring.
The docstring is generic. Consider describing the three-valued logic functionality specifically.
class QubeTVL:
- """Mix-in class providing operations for Qube objects."""
+ """Mix-in class providing three-valued logic operations for Qube objects."""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class QubeTVL: | |
| """Mix-in class providing operations for Qube objects.""" | |
| class QubeTVL: | |
| """Mix-in class providing three-valued logic operations for Qube objects.""" |
🤖 Prompt for AI Agents
In polymath/extensions/tvl.py around lines 8 to 9, the class docstring is
generic; update it to explicitly describe the three-valued logic functionality
this mix-in provides (e.g., the three truth values it models, semantics for
unknown/partial values, and how Qube objects should use the mix-in). Mention the
main public methods/behaviour (inputs, return values, and side effects), any
invariants or expectations on Qube attributes, and a short usage example or note
about interoperability with boolean operations.
| class QubeVectorOps: | ||
| """Mix-in class providing operations for Qube objects.""" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Class docstring is too generic.
The docstring "Mix-in class providing operations for Qube objects" doesn't describe what operations. Consider something like "Mix-in class providing vector and aggregation operations (dot, cross, norm, mean, sum, etc.) for Qube objects."
🤖 Prompt for AI Agents
In polymath/extensions/vector_ops.py around lines 10 to 11, the class docstring
is too generic; replace it with a specific summary of what operations the mix-in
provides (e.g., vector and aggregation operations such as dot, cross, norm,
normalize, magnitude, mean, sum, min, max, and any broadcasting/shape
behaviors), mention the target type (Qube objects), and briefly note any
important behavior or exceptions (e.g., axis handling or in-place vs.
functional). Keep it concise, use imperative/descriptive tone suitable for API
docs, and follow existing project docstring style (one-line summary plus a short
elaboration if needed).
| for i in axis: | ||
| try: | ||
| _ = selections[i] | ||
| except IndexError: | ||
| raise IndexError(f'axis is out of range ({-self._ndims},{self._ndims}) in ' | ||
| f'{type(self)}.{op}: {i}') | ||
|
|
||
| if selections[i]: | ||
| raise IndexError(f'duplicated axis in {type(self)}.{op}: {axis_for_show}') | ||
|
|
||
| def _check_axis(arg, axis, op): | ||
| """Validate the axis as None, an int, or a tuple of ints. | ||
| selections[i] = True |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use exception chaining for clearer tracebacks.
When re-raising exceptions inside an except block, use raise ... from err or raise ... from None to distinguish between errors in exception handling and the original error.
try:
_ = selections[i]
except IndexError:
- raise IndexError(f'axis is out of range ({-self._ndims},{self._ndims}) in '
- f'{type(self)}.{op}: {i}')
+ raise IndexError(f'axis is out of range ({-self._ndims},{self._ndims}) in '
+ f'{type(self)}.{op}: {i}') from None🧰 Tools
🪛 Ruff (0.14.7)
138-139: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
138-139: Avoid specifying long messages outside the exception class
(TRY003)
142-142: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In polymath/extensions/vector_ops.py around lines 134 to 144, the code catches
IndexError but re-raises a new IndexError without chaining; change the except
block to "except IndexError as err:" and re-raise the new IndexError using
"raise IndexError(... ) from err" so the original IndexError is preserved in the
traceback (leave the duplicated-axis check as-is).
Summary by CodeRabbit
Refactor
Chore
✏️ Tip: You can customize this high-level summary in your review settings.