Skip to content

chore: fix type checking issues#700

Open
mayeut wants to merge 3 commits into
pypa:mainfrom
mayeut:fix-type-checking
Open

chore: fix type checking issues#700
mayeut wants to merge 3 commits into
pypa:mainfrom
mayeut:fix-type-checking

Conversation

@mayeut
Copy link
Copy Markdown
Member

@mayeut mayeut commented May 30, 2026

pyelftools recently released a version with typing hints.
auditwheel fails to pass mypy checks with this new version.

This PR aims to fix (or ignore) mypy reported issues.

Comment on lines +25 to +26
if TYPE_CHECKING:
assert section is None or isinstance(section, type_)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be made a runtime check but this way, we ensure the exact same runtime behaviour as before.

msg = f"Could not find soname in {fn}"
raise ValueError(msg)
needed.extend(t.needed for t in section.iter_tags() if t.entry.d_tag == "DT_NEEDED")
needed.extend(t.needed for t in section.iter_tags() if t.entry.d_tag == "DT_NEEDED") # type: ignore[attr-defined]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the # type: ignore[attr-defined] are for dynamically defined attributes

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates auditwheel’s code and integration tests to satisfy mypy when used with newer pyelftools versions that ship richer type hints.

Changes:

  • Add explicit DynamicSection / segment type narrowing (isinstance(...)) where .dynamic sections and PT_* segments are accessed.
  • Add targeted # type: ignore[attr-defined] annotations for pyelftools tag attributes not represented in stubs.
  • Update the pre-commit mypy hook environment to use pyelftools>=0.33.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/integration/test_manylinux.py Narrows .dynamic section type for mypy and ignores an attribute not present in stubs.
tests/integration/test_android.py Narrows .dynamic section type for mypy before iterating dynamic tags.
src/auditwheel/lddtree.py Narrows segment types (InterpSegment, DynamicSegment) and ignores tag attributes missing from stubs.
src/auditwheel/elfutils.py Adds a typed helper for section lookup and narrows section types for mypy.
.pre-commit-config.yaml Ensures mypy runs against a pyelftools version that includes the new typing hints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/auditwheel/elfutils.py Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.74%. Comparing base (dbc2208) to head (dfb4a75).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #700      +/-   ##
==========================================
+ Coverage   95.73%   95.74%   +0.01%     
==========================================
  Files          23       23              
  Lines        1923     1928       +5     
  Branches      362      361       -1     
==========================================
+ Hits         1841     1846       +5     
  Misses         46       46              
  Partials       36       36              

☔ View full report in Codecov by Sentry.
📢 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.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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