Migrate IR and PxdWriter to headerkit#61
Conversation
Replace duplicated IR classes, PxdWriter, Cython type registries, and keyword lists with thin shim modules that re-export from headerkit. The shims provide 100% backward-compatible imports so no consuming code needs changes. The ir_writer shim subclasses headerkit's PxdWriter with stub_cimport_prefix="autopxd.stubs" pre-configured, preserving autopxd2's stub cimport behavior. Update PycparserBackend.parse() signature to match headerkit's ParserBackend protocol (accepts keyword-only params it doesn't use). Delete test_ir.py (24 tests) and test_cython_types.py (15 tests) which are now maintained in headerkit. All 359 remaining tests pass.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a major refactoring that migrates significant parts of the codebase (IR, PxdWriter, type registries) to a new external library, headerkit. This greatly reduces code duplication and centralizes the logic. The changes are well-structured, creating shim modules to maintain backward compatibility.
My review focuses on a few points related to dependency management and reliance on private APIs, which could affect the project's stability and build reproducibility. Overall, this is an excellent cleanup.
| ] | ||
| requires-python = ">=3.10" | ||
| dependencies = ["Click", "pycparser"] | ||
| dependencies = ["Click", "pycparser", "headerkit"] |
There was a problem hiding this comment.
The headerkit dependency is not pinned to the specific development version required by this PR, which is inconsistent with .pre-commit-config.yaml and will likely cause installation issues for anyone trying to build from this branch. To ensure the project is buildable, please pin it to the same git branch/commit.
| dependencies = ["Click", "pycparser", "headerkit"] | |
| dependencies = ["Click", "pycparser", "headerkit @ git+https://github.com/axiomantic/headerkit.git@elijahr/autopxd2-migration"] |
| additional_dependencies: | ||
| - pycparser | ||
| - click | ||
| - headerkit @ git+https://github.com/axiomantic/headerkit.git@v0.6.0 |
There was a problem hiding this comment.
For better stability and reproducibility, it's recommended to pin this dependency to a specific commit hash instead of a branch name. Branches can be updated or deleted, which could break the pre-commit setup for others or in the future.
| from headerkit.writers._cython_types import ( | ||
| CYTHON_STDLIB_HEADERS, | ||
| CYTHON_STDLIB_TYPES, | ||
| HEADERKIT_STUB_TYPES, | ||
| LIBCPP_HEADERS, | ||
| LIBCPP_TYPES, | ||
| get_cython_module_for_type, | ||
| get_libcpp_module_for_type, | ||
| get_stub_module_for_type, | ||
| ) |
There was a problem hiding this comment.
Importing from a private module (_cython_types) can be fragile as its API is not guaranteed to be stable across headerkit versions. It would be more robust to import these from a public API within headerkit if possible. Consider exposing these symbols in headerkit.writers.cython or a similar public module.
| from headerkit.writers._cython_keywords import ( | ||
| C_keywords, | ||
| cython_keywords, | ||
| keywords, | ||
| ) |
There was a problem hiding this comment.
Similar to cython_types.py, this module imports from a private headerkit module (_cython_keywords). This creates a dependency on headerkit's internal implementation, which might change unexpectedly. It's recommended to rely on public APIs. Please consider exposing these keywords through a public module in headerkit.
BREAKING CHANGE: Remove autopxd.cython_types and autopxd.keywords modules (zero consumers after migration to headerkit shims). Remove dead elif/else branches in translate() for backends not supporting full ParserBackend protocol (both backends now do). Remove unused IGNORE_DECLARATIONS and STDINT_DECLARATIONS from declarations.py. Fix stale test reference in contributing docs. Bump to 4.0.0.
BREAKING CHANGE: The pycparser backend has been removed. libclang is now the only parser backend. Install libclang via your system package manager or headerkit's install_libclang tool. Remove pycparser_backend.py, declarations.py, regenerate_stubs.py, and all pycparser fake header stubs (stubs/include/, stubs/darwin-include/). Remove pycparser from runtime dependencies. Simplify resolve_backend() and CLI: --backend only accepts "auto" or "libclang". Remove validate_libclang_options() (moot with single backend). Remove FALLBACK_WARNING (no pycparser fallback). Convert all pycparser-specific tests to use libclang backend. Update all documentation to reflect single-backend architecture.
The expected file was written for the old pycparser-based writer. The headerkit CythonWriter produces a different (correct) ordering: forward declarations and typedefs first, then enums with cdef instead of ctypedef, globals before struct bodies, ctypedef struct for Buffer, and parameter names stripped from function pointer typedefs.
Configure Dependabot for pip and github-actions ecosystems with weekly schedules and grouped minor/patch updates. Add a scheduled workflow to run pre-commit autoupdate weekly and create PRs for hook version bumps.
- Add .github/dependabot.yml for pip and github-actions ecosystems - Add pre-commit autoupdate workflow (weekly, via peter-evans/create-pull-request) - Bump headerkit dependency to >=0.6.1,<1.0.0 - Update pre-commit mypy headerkit pin to v0.6.1
1 participant
Summary
stub_cimport_prefix="autopxd.stubs"to preserve autopxd2's stub cimport behaviorDependency
Requires headerkit v0.6.0 (PR pending).
Test plan