Skip to content

Address review feedback for numeric parse edge cases in parse_util#587

Closed
fabiocaccamo with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-comments-in-review-thread
Closed

Address review feedback for numeric parse edge cases in parse_util#587
fabiocaccamo with Copilot wants to merge 1 commit into
mainfrom
copilot/fix-comments-in-review-thread

Conversation

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Describe your changes
This updates the PR to resolve all comments from the linked review thread by tightening parse_int error handling for float-fallback edge cases and aligning parse_float typing with its supported inputs.

  • parse_int robustness

    • Keep float-string fallback (int(float(val))) for values like "3.5".
    • Return None (instead of raising) for overflow/non-finite float conversions (e.g. "inf", "1e309"), so caller defaults still apply.
  • parse_float typing alignment

    • Update the function signature to reflect real accepted inputs (bool | float | int | str) to match runtime behavior and avoid misleading type hints.
  • Tests alignment

    • Ensure parse-dict expectations cover the corrected bool and float-like string conversion behavior.
def _parse_int(val: str) -> int | None:
    try:
        return int(val)
    except ValueError:
        try:
            return int(float(val))
        except (ValueError, OverflowError):
            return None

def parse_float(val: bool | float | int | str) -> float | None:
    if isinstance(val, (bool, int)):
        return float(val)
    return _parse_with(val, type_util.is_float, _parse_float)

Related issue
Addresses the review comments in PR #578 review thread (pullrequestreview-4629196338).

Checklist before requesting a review

  • I have performed a self-review of my code.
  • I have added tests for the proposed changes.
  • I have run the tests and there are not errors.

Copilot AI changed the title [WIP] Fix code as per all comments in review thread Address review feedback for numeric parse edge cases in parse_util Jul 7, 2026
Copilot AI requested a review from fabiocaccamo July 7, 2026 13:13
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.33%. Comparing base (f99b35a) to head (de6ec1b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #587   +/-   ##
=======================================
  Coverage   98.33%   98.33%           
=======================================
  Files          64       64           
  Lines        2399     2399           
=======================================
  Hits         2359     2359           
  Misses         40       40           
Flag Coverage Δ
unittests 98.33% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants