fix: resolve TypeError in SPI/UART tests and fix classmethod_ property chaining - #287
Open
Shikhar-Kesharwani wants to merge 1 commit into
Open
Conversation
- Remove sys.version_info check in classmethod_ descriptor so that properties chained with classmethod_ correctly return evaluated values on Python 3.11+ instead of bound method objects. - Fix typo PWM_FERQUENCY -> PWM_FREQUENCY in test_spi.py and test_uart.py. - Add regression tests verifying numeric float/int property evaluation and arithmetic frequency calculations. Fixes fossasia#270
Reviewer's GuideUpdates the custom classmethod descriptor to correctly evaluate chained properties on both classes and instances across Python versions, then fixes SPI/UART test frequency naming and adds regression coverage for numeric property values and dynamic PWM calculations. Sequence diagram for chained SPI and UART property evaluationsequenceDiagram
participant Test
participant SPIMaster
participant UART
participant classmethod_
participant property
Test->>SPIMaster: access _frequency
SPIMaster->>classmethod_: __get__(obj, cls)
classmethod_ ->> property: __get__(cls)
property-->>Test: numeric _frequency
Test->>Test: calculate _frequency * 2 / 3
Test->>UART: access _baudrate
UART->>classmethod_: __get__(obj, cls)
classmethod_ ->> property: __get__(cls)
property-->>Test: numeric _baudrate
Test->>Test: calculate _baudrate // 2
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change removes a Python-version guard from property-wrapped class method access. It also corrects the ChangesDescriptor access and frequency validation
Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to The descriptor access fix and corrected SPI/UART frequency tests address the reported TypeErrors while preserving dynamic calculations. Current validation shows no merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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.
Fixes #270
Summary
When collecting or running
tests/test_spi.pyandtests/test_uart.pyon Python 3.11+, pytest raisedTypeError: unsupported operand type(s)due to arithmetic onSPIMaster._frequencyandUART._baudrate.Root Cause
pslab/bus/__init__.py,classmethod_.__get__guarded the evaluation of property descriptors behindif sys.version_info < (3, 9) and isinstance(self.f, property):. In Python 3.11+, chainingclassmethodon descriptors was deprecated and in Python 3.13 it was completely removed. As a result,super().__get__(obj, cls)returned a bound method object instead of evaluating the property getter.PWM_FERQUENCYwas misspelled in bothtests/test_spi.pyandtests/test_uart.py.Changes
classmethod_.__get__inpslab/bus/__init__.pyto always evaluateself.f.__get__(cls)whenself.fis an instance ofproperty, ensuring full compatibility across Python versions (3.8 through 3.14+) on both classes and instances. Removed unusedsysimport.PWM_FERQUENCY->PWM_FREQUENCYacrosstests/test_spi.pyandtests/test_uart.py.SPIMaster._frequency * 2 / 3andUART._baudrate // 2) rather than hardcoding frequencies.tests/test_spi.pyandtests/test_uart.pyverifying numeric property evaluation and dynamic frequency calculations.Testing
pytest tests/test_spi.py tests/test_uart.py --collect-only(all 25 tests collected successfully)pytest tests/test_spi.py tests/test_uart.py -k "test_frequency or test_clock or test_pwm or test_baudrate"(5 passed)flake8 pslab/bus/__init__.py tests/test_spi.py tests/test_uart.py(clean, 0 warnings)git diff --check(clean, 0 whitespace errors)Summary by Sourcery
Restore reliable SPI and UART frequency property behavior across Python versions and strengthen regression coverage.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit
Bug Fixes
Tests