Skip to content

Address review comments on PR #578: catch OverflowError in _parse_int and fix parse_float annotation - #584

Merged
fabiocaccamo merged 3 commits into
mainfrom
copilot/fix-comments-from-review-thread
Jul 7, 2026
Merged

Address review comments on PR #578: catch OverflowError in _parse_int and fix parse_float annotation#584
fabiocaccamo merged 3 commits into
mainfrom
copilot/fix-comments-from-review-thread

Conversation

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Describe your changes

Addresses both comments in the review thread on PR #578. Since #578 comes from a fork, its commit was cherry-picked onto this branch first, then the review fixes applied on top.

  • _parse_int overflow safety — the int(float(val)) fallback introduced in fix: parse_float(bool/int) and parse_int(float-string) return wrong results #578 could raise OverflowError for inputs like "inf" or "1e309" (where float(...) yields infinity), breaking get_int(...) callers instead of falling back to the default. The fallback now catches (ValueError, OverflowError) and returns None.
  • parse_float type annotation — signature updated from val: str to val: bool | float | int | str to reflect the input types it actually accepts.
def _parse_int(val: str) -> int | None:
    try:
        return int(val)
    except ValueError:
        try:
            return int(float(val))
        except (ValueError, OverflowError):
            return None

Related issue
?

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.

gaoflow and others added 2 commits July 7, 2026 10:41
…s to int

- `get_float(True)` wrongly returned `0.0` (the default) because `bool`
  failed the `is_float` type-check, `str(True)` produced `"True"`, and
  `float("True")` raised `ValueError`.  Now `parse_float` short-circuits
  on any `bool`/`int` value and returns `float(val)` directly, so
  `get_float(True)` → `1.0` and `get_float(False)` → `0.0`.

- `get_int("3.5")` wrongly returned the caller-supplied default instead
  of `3` because `int("3.5")` raises `ValueError`.  `_parse_int` now
  falls back to `int(float(val))` so numeric strings with a fractional
  part are truncated correctly.

- `get_int(True)` returned `True` (a `bool`) rather than the `int` `1`
  because `isinstance(True, int)` is `True` and `_parse_with` returned
  the value unchanged.  `parse_int` now explicitly casts `bool` inputs
  with `int(val)` before any further processing.
Copilot AI changed the title [WIP] Fix code based on review comments Address review comments on PR #578: catch OverflowError in _parse_int and fix parse_float annotation Jul 7, 2026
Copilot AI requested a review from fabiocaccamo July 7, 2026 10:45
@fabiocaccamo
fabiocaccamo marked this pull request as ready for review July 7, 2026 11:01
@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 (1c510aa) to head (52a5f5d).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #584   +/-   ##
=======================================
  Coverage   98.32%   98.33%           
=======================================
  Files          64       64           
  Lines        2392     2399    +7     
=======================================
+ Hits         2352     2359    +7     
  Misses         40       40           
Flag Coverage Δ
unittests 98.33% <100.00%> (+<0.01%) ⬆️

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.

@fabiocaccamo
fabiocaccamo merged commit f3bc063 into main Jul 7, 2026
21 checks passed
@fabiocaccamo
fabiocaccamo deleted the copilot/fix-comments-from-review-thread branch July 14, 2026 14:31
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.

3 participants