Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Apr 25, 2025

PR Type

Enhancement, Tests


Description

Add np.void support in comparator
Check dtype before comparing fields
Recursively compare each record field
Add tests for np.void comparisons


Changes walkthrough 📝

Relevant files
Enhancement
comparator.py
Enable numpy.void record comparisons                                         

codeflash/verification/comparator.py

  • Add np.void comparison branch
  • Return False on dtype mismatch
  • Recursively compare each record field
  • +5/-0     
    Tests
    test_comparator.py
    Add tests for numpy.void comparator                                           

    tests/test_comparator.py

  • Define structured dtype for tests
  • Extract np.void instances for comparison
  • Assert comparator on equal and unequal records
  • +14/-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

    Opaque void support

    The new np.void comparator assumes orig.dtype.fields is iterable. For non-structured np.void types, dtype.fields can be None, causing a TypeError. Consider adding a guard or fallback for opaque void types.

    if HAS_NUMPY and isinstance(orig, np.void):
        if orig.dtype != new.dtype:
            return False
        return all(comparator(orig[field], new[field], superset_obj) for field in orig.dtype.fields)
    Test coverage

    Current tests only cover simple structured np.void instances. Add tests for opaque byte-blob voids (where dtype.fields is None), nested record dtypes, and scenarios with superset_obj=True/False.

    dt = np.dtype([('name', 'S10'), ('age', np.int32)])
    a_struct = np.array([('Alice', 25)], dtype=dt)
    b_struct = np.array([('Alice', 25)], dtype=dt)
    c_struct = np.array([('Bob', 30)], dtype=dt)
    
    a_void = a_struct[0]
    b_void = b_struct[0]
    c_void = c_struct[0]
    
    assert isinstance(a_void, np.void)
    assert comparator(a_void, b_void)
    assert not comparator(a_void, c_void)

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Compare bytes for empty fields

    For structured dtypes without named fields, compare raw byte content via tobytes().

    codeflash/verification/comparator.py [151-154]

     if HAS_NUMPY and isinstance(orig, np.void):
         if orig.dtype != new.dtype:
             return False
    +    if not orig.dtype.fields:
    +        return orig.tobytes() == new.tobytes()
         return all(comparator(orig[field], new[field], superset_obj) for field in orig.dtype.fields)
    Suggestion importance[1-10]: 7

    __

    Why: Adding a check for non-structured dtypes avoids iterating over None and correctly compares raw byte content for simple void types.

    Medium
    Use dtype.names for iteration

    Iterate over orig.dtype.names instead of dtype.fields for deterministic field order.

    codeflash/verification/comparator.py [154]

    -return all(comparator(orig[field], new[field], superset_obj) for field in orig.dtype.fields)
    +return all(comparator(orig[name], new[name], superset_obj) for name in orig.dtype.names)
    Suggestion importance[1-10]: 5

    __

    Why: Iterating over dtype.names ensures a deterministic field order and is more semantically appropriate than using .fields directly.

    Low
    Possible issue
    Check new is np.void

    Add a guard to ensure new is also an instance of np.void before comparing fields.

    codeflash/verification/comparator.py [151-154]

     if HAS_NUMPY and isinstance(orig, np.void):
    +    if not isinstance(new, np.void):
    +        return False
         if orig.dtype != new.dtype:
             return False
         return all(comparator(orig[field], new[field], superset_obj) for field in orig.dtype.fields)
    Suggestion importance[1-10]: 6

    __

    Why: The guard ensures symmetry by verifying new is also a np.void, preventing invalid comparisons when types differ.

    Low

    @KRRT7 KRRT7 requested a review from misrasaurabh1 April 25, 2025 03:00
    @KRRT7 KRRT7 enabled auto-merge April 25, 2025 03:07
    @KRRT7 KRRT7 merged commit 6dbfa20 into main Apr 25, 2025
    16 checks passed
    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.

    3 participants