Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented May 18, 2025

User description

Days output when converting from nanoseconds


PR Type

Enhancement, Tests


Description

  • Add days unit support in humanize_runtime

  • Explicitly handle "hour" and "hours" units

  • Add tests for singular and plural days


Changes walkthrough 📝

Relevant files
Enhancement
time_utils.py
Support days conversion in humanize_runtime                           

codeflash/code_utils/time_utils.py

  • Change hours branch to elif units in {"hour","hours"}
  • Add else branch handling "day" and "days"
  • Compute days via time_micro/(24*3600*1000**2)
  • +3/-2     
    Tests
    test_humanize_time.py
    Test day(s) conversion                                                                     

    tests/test_humanize_time.py

  • Add test for singular "day" output
  • Add test for plural "days" output
  • +2/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Explicit Days Branch

    Use an explicit elif units in {"day", "days"} for days conversion instead of a catch-all else to prevent unintended unit matches.

    elif units in {"hour", "hours"}: #hours
        runtime_human = "%.3g" % (time_micro / (3600 * 1000**2))
    else: #days
        runtime_human = "%.3g" % (time_micro / (24*3600 * 1000**2))
    Edge Case Missing

    Add tests for an exact 1-day duration (e.g., 86,400 seconds in nanoseconds) to verify correct singular "day" output and formatting.

    assert humanize_runtime(98765431298760) == "1.14 day"
    assert humanize_runtime(197530862597520) == "2.29 days"

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Explicitly handle days unit

    Replace the generic else with an explicit days check and add a final fallback to
    error on unexpected units. This ensures only "day"/"days" are handled in this branch
    and guards against future unit mismatches.

    codeflash/code_utils/time_utils.py [29-30]

    -else: #days
    -    runtime_human = "%.3g" % (time_micro / (24*3600 * 1000**2))
    +elif units in {"day", "days"}:  # days
    +    runtime_human = "%.3g" % (time_micro / (24 * 3600 * 1000**2))
    +else:
    +    raise ValueError(f"Unsupported unit: {units}")
    Suggestion importance[1-10]: 7

    __

    Why: This change replaces a catch-all else with a specific elif units in {"day", "days"} and adds a fallback ValueError, preventing unexpected unit values from being silently treated as days.

    Medium

    @aseembits93 aseembits93 requested a review from misrasaurabh1 May 18, 2025 20:52
    @KRRT7
    Copy link
    Contributor

    KRRT7 commented May 18, 2025

    while you're here, add a console.rule() in codeflash/discovery/functions_to_optimize.py before this block

                logger.info(
                     f"It might take about {humanize_runtime(functions_count*three_min_in_ns)} to fully optimize this project. Codeflash "
                     f"will keep opening pull requests as it finds optimizations."
                 )

    @aseembits93 aseembits93 enabled auto-merge May 19, 2025 01:04
    @aseembits93 aseembits93 merged commit 78e2d65 into main May 19, 2025
    15 checks passed
    @KRRT7 KRRT7 deleted the humanize-runtime-days branch May 19, 2025 02:29
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    4 participants