Refactor: Replace magic numbers with constants in ScienceLab

[TejasAnalyst]

his PR addresses a 5-year-old TODO in sciencelab.py. I have replaced hardcoded magic numbers in temperature and _get_ctmu_voltage with named constants and a calibration dictionary to improve code maintainability.

Summary by Sourcery

Refactor ScienceLab ADC and temperature handling to replace hardcoded magic numbers with shared constants and a calibration table.

Enhancements:

• Introduce shared ADC hardware constants for maximum voltage and resolution and use them in CTMU voltage calculations.
• Replace inline temperature calibration formulas with a dictionary-based calibration lookup for different current sources.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors ScienceLab to replace inline magic numbers in temperature calculation and CTMU ADC conversion with named constants and a calibration dictionary, improving readability and maintainability without changing behavior.

Class diagram for ScienceLab temperature and CTMU refactor

classDiagram
    class ScienceLabModule {
        <<module>>
        +float ADC_VMAX
        +int ADC_RESOLUTION
        +dict TEMP_CALIB
    }

    class ScienceLab {
        +temperature float
        +_get_ctmu_voltage(channel int, current_range int, tgen bool) float
    }

    ScienceLabModule <.. ScienceLab : uses

    class TEMP_CALIB_entry {
        +float offset
        +float slope
    }

    ScienceLabModule "1" --> "*" TEMP_CALIB_entry : maps_current_source

Loading

File-Level Changes

Change	Details	Files
Introduce named constants and calibration mapping for hardware-related magic numbers.	Add ADC_VMAX and ADC_RESOLUTION module-level constants for ADC configuration. Add TEMP_CALIB dictionary mapping current source values to temperature calibration offset and slope. Update temperature property to compute temperature using TEMP_CALIB instead of hardcoded conditionals. Update _get_ctmu_voltage to use ADC_VMAX and ADC_RESOLUTION instead of local literals 3.3 and 12.	pslab/sciencelab.py

Possibly linked issues

• #N/A: PR centralizes and names CTMU temperature calibration and ADC constants, matching the issue’s refactor-and-clarify scope without behavior change.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In temperature, using TEMP_CALIB.get(cs) without a fallback will raise an exception if cs ever changes to an unsupported value; consider either validating cs or using a default/explicit error branch to make failures clearer.
• The new constants (ADC_VMAX, ADC_RESOLUTION, TEMP_CALIB) are defined at module level; if these are tightly coupled to ScienceLab, consider making them class attributes or namespacing them to avoid accidental reuse or modification elsewhere.
• The inline comments in TEMP_CALIB are currently just # placeholders; either remove them or replace them with brief descriptions (e.g., units or source of calibration) to clarify the meaning of offset and slope.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `temperature`, using `TEMP_CALIB.get(cs)` without a fallback will raise an exception if `cs` ever changes to an unsupported value; consider either validating `cs` or using a default/explicit error branch to make failures clearer.
- The new constants (`ADC_VMAX`, `ADC_RESOLUTION`, `TEMP_CALIB`) are defined at module level; if these are tightly coupled to `ScienceLab`, consider making them class attributes or namespacing them to avoid accidental reuse or modification elsewhere.
- The inline comments in `TEMP_CALIB` are currently just `#` placeholders; either remove them or replace them with brief descriptions (e.g., units or source of calibration) to clarify the meaning of `offset` and `slope`.

## Individual Comments

### Comment 1
<location> `pslab/sciencelab.py:66-67` </location>
<code_context>
-            return (760 - V * 1000) / 1.56  # current source = 3
+        
+        # Clean lookup from global constants
+        cal = TEMP_CALIB.get(cs)
+        return (cal["offset"] - V * 1000) / cal["slope"]

     def _get_ctmu_voltage(self, channel: int, current_range: int, tgen: bool = True):
</code_context>

<issue_to_address>
**issue (bug_risk):** Consider handling the case where `cs` is not present in `TEMP_CALIB` to avoid a runtime error.

Because `cs` is currently hard-coded to 3 this is safe today, but the new dict lookup changes previously exhaustive handling into something that can return `None`, which would cause a `TypeError` when subscripting `cal`. Using `TEMP_CALIB[cs]` (letting a `KeyError` surface) or adding explicit validation/fallback (e.g., clear error or default calibration) would make failures more predictable if `cs` is later parameterized or modified elsewhere.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[TejasAnalyst]

I have updated the code to include a safety check for the current source lookup, as suggested by the Sourcery AI review. This ensures the function handles unsupported current sources gracefully by raising a ValueError.