Docs: Add docstring to _calibrate method in AnalogInput#264
Docs: Add docstring to _calibrate method in AnalogInput#264vabhravi wants to merge 2 commits intofossasia:mainfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds documentation for the File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The docstring for
_calibratecould mention that it updates the internal_scaleand_unscalecallables as a side effect so future readers understand why it doesn’t return anything. - The whitespace-only change in
_range_normalize(removing and re-adding the same return line) is unnecessary and can be dropped to keep the diff minimal.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The docstring for `_calibrate` could mention that it updates the internal `_scale` and `_unscale` callables as a side effect so future readers understand why it doesn’t return anything.
- The whitespace-only change in `_range_normalize` (removing and re-adding the same return line) is unnecessary and can be dropped to keep the diff minimal.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Small process note. We have automatic Copilot PR reviews enabled on this repository. These reviews are only triggered if the contributor has GitHub Copilot enabled and an active license on their own account. Please enable Copilot in your GitHub settings if you have access. In many regions, free licenses are available through educational institutions or developer programs. Enabling Copilot helps us speed up the auto review process and reduces manual review overhead for the core team. The team will review your PR. Thank you for your contribution and cooperation. |
|
Please document the changes and show the functionality in a screencast to show familiarity with the feature. |
There was a problem hiding this comment.
Pull request overview
This PR intends to improve readability by documenting how AnalogInput._calibrate derives coefficients used to convert between raw ADC readings and voltages.
Changes:
- Added a docstring to
AnalogInput._calibrate. - (Unintended) Changed indentation/scope of
_calibrateand_range_normalize, which currently breaks the module.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _calibrate(self): | ||
| """ | ||
| Calculates the scaling coefficients based on current gain and resolution. | ||
|
|
||
| This updates the internal _scale and _unscale callables as a side effect, | ||
| preparing them to convert between raw integer ADC values and floating | ||
| point voltages. | ||
| """ | ||
| A = INPUT_RANGES[self._name][0] / self._gain | ||
| B = INPUT_RANGES[self._name][1] / self._gain | ||
| slope = B - A | ||
| intercept = A | ||
| self._scale = np.poly1d([slope / self._resolution, intercept]) | ||
| self._unscale = np.poly1d( |
There was a problem hiding this comment.
The _calibrate method is no longer indented as a class member, and its docstring/body are also misindented. As written this will raise an IndentationError/break the class structure. Re-indent def _calibrate to align with other AnalogInput methods (4 spaces), and indent its docstring/body accordingly.
| def _calibrate(self): | |
| """ | |
| Calculates the scaling coefficients based on current gain and resolution. | |
| This updates the internal _scale and _unscale callables as a side effect, | |
| preparing them to convert between raw integer ADC values and floating | |
| point voltages. | |
| """ | |
| A = INPUT_RANGES[self._name][0] / self._gain | |
| B = INPUT_RANGES[self._name][1] / self._gain | |
| slope = B - A | |
| intercept = A | |
| self._scale = np.poly1d([slope / self._resolution, intercept]) | |
| self._unscale = np.poly1d( | |
| def _calibrate(self): | |
| """ | |
| Calculates the scaling coefficients based on current gain and resolution. | |
| This updates the internal _scale and _unscale callables as a side effect, | |
| preparing them to convert between raw integer ADC values and floating | |
| point voltages. | |
| """ | |
| A = INPUT_RANGES[self._name][0] / self._gain | |
| B = INPUT_RANGES[self._name][1] / self._gain | |
| slope = B - A | |
| intercept = A | |
| self._scale = np.poly1d([slope / self._resolution, intercept]) | |
| self._unscale = np.poly1d( |
| def _range_normalize(self, x: np.ndarray, norm: int = 1) -> np.ndarray: | ||
| """Normalize waveform table to the digital output range.""" | ||
| x = (x - self.RANGE[0]) / (self.RANGE[1] - self.RANGE[0]) * norm | ||
| return np.int16(np.round(x)).tolist() No newline at end of file |
There was a problem hiding this comment.
_range_normalize appears to have been dedented to module scope (it now starts at column 0 right after a method return). This breaks the AnalogOutput class and will also likely cause an IndentationError in the file. _range_normalize should remain an instance method of AnalogOutput (indented under the class, consistent with other methods).
| def _range_normalize(self, x: np.ndarray, norm: int = 1) -> np.ndarray: | |
| """Normalize waveform table to the digital output range.""" | |
| x = (x - self.RANGE[0]) / (self.RANGE[1] - self.RANGE[0]) * norm | |
| return np.int16(np.round(x)).tolist() | |
| def _range_normalize(self, x: np.ndarray, norm: int = 1) -> np.ndarray: | |
| """Normalize waveform table to the digital output range.""" | |
| x = (x - self.RANGE[0]) / (self.RANGE[1] - self.RANGE[0]) * norm | |
| return np.int16(np.round(x)).tolist() |
| def _calibrate(self): | ||
| """ |
There was a problem hiding this comment.
PR description indicates this is a docstring-only change, but this hunk also changes indentation/scope for _calibrate (and later _range_normalize), which alters runtime behavior and currently breaks the module. If the intent is documentation only, please revert whitespace changes and keep method indentation unchanged.
2 participants
I noticed the _calibrate method in analog.py was missing a docstring. I added documentation to explain how it calculates slope and intercept for voltage conversion, improving code readability.
Summary by Sourcery
Documentation: