Fix ArrayWrapper inlining to go to pyx only, not pxd

[timosachsenberg]

Summary

This PR fixes ArrayWrapper inlining so that ArrayWrapper classes are placed only in .pyx files, not .pxd files. This prevents Cython declaration conflicts when projects have their own ArrayWrapper definitions.

Changes

• Re-enable ArrayWrapper inlining (was disabled in previous commit)
• ArrayWrappers now go to top_level_pyx_code instead of top_level_code
• When write_pxd=True, this ensures ArrayWrappers only appear in pyx files

Benefits for downstream projects

Projects like OpenMS can now:

1. Remove their own ArrayWrapper definitions from addon files
2. Use autowrap's more complete ArrayWrappers with:
   • Additional integer array types (Int8, Int16, Int32, Int64, UInt8, UInt16, UInt32, UInt64)
   • Extra methods: __init__(size), resize(), size()
   • Better documentation

Testing

• Tested with OpenMS pyopenms build
• All pyopenms unit tests pass
• MSExperiment.get2DPeakDataLong() works correctly with autowrap's ArrayWrappers

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed potential conflicts with existing ArrayWrapper definitions by adjusting where generated wrapper code is emitted.

• Documentation

  • Updated documentation comment in hash implementation reference.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR relocates ArrayWrapper code generation from top-level code to pyx-specific top-level code in the CodeGenerator, reducing potential conflicts with pre-existing ArrayWrapper definitions. Minor documentation and formatting adjustments are also made.

Changes

Cohort / File(s)	Summary
Code Generation Logic autowrap/CodeGenerator.py	ArrayWrapper definitions redirected from top_level_code to top_level_pyx_code in inline_array_wrappers and import handling paths; comment clarifies ArrayWrappers should reside in pyx files. Hash method docstring updated (documentation only). Minor formatting adjustments in import section.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• jpfeuffer

Poem

🐰 Hoppy refactoring, code takes flight,
ArrayWrappers now in the pyx so right,
Conflicts fade like clover in the sun,
Cleaner generation—beautifully done!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: ArrayWrappers are moved from being emitted to both pyx and pxd files to pyx only, which is exactly what the changeset implements.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2faef85 and 11e5f21.

📒 Files selected for processing (1)

• autowrap/CodeGenerator.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

• GitHub Check: test (==3.1.0, 3.10)
• GitHub Check: test (==3.2.0, 3.12)
• GitHub Check: test (==3.2.0, 3.10)
• GitHub Check: test (==3.2.0, 3.13)
• GitHub Check: test (==3.1.0, 3.13)
• GitHub Check: test (==3.1.0, 3.11)
• GitHub Check: test (==3.1.0, 3.12)
• GitHub Check: test (==3.2.0, 3.11)

🔇 Additional comments (4)

autowrap/CodeGenerator.py (4)

681-681: Documentation fix - comment now accurately reflects implementation.

The comment correctly references $cy_type which is used in the cpp_hash[$cy_type] declaration on line 682, improving code documentation accuracy.

---

2136-2140: LGTM - Array wrapper inlining properly integrated.

The conditional call to inline_array_wrappers() is appropriately placed in the cimports setup flow, ensuring ArrayWrapper classes are inlined only when numpy support is enabled.

---

2178-2181: Core PR objective successfully implemented - ArrayWrappers now pyx-only.

Changing from top_level_code to top_level_pyx_code ensures ArrayWrapper classes are emitted exclusively to .pyx files, preventing declaration conflicts in .pxd files when downstream projects provide their own ArrayWrapper implementations. The added comments clearly document this intentional behavior.

---

2089-2091: Minor formatting improvement - no functional change.

The import statement is split across two lines for better readability. This is purely cosmetic and doesn't affect functionality.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jpfeuffer]

I guess if the tests pass it's fine. I am wondering if it is good to have in every pyx though. Might increase the compile times. I think that was the idea of having them in the Pxd to be shared across compiled modules. But I am also not understanding it completely.
An additional option that I never got to work is to actually have separate Pxd/pyx files for this to not have to inline it into the code that we are generating.

[jpfeuffer]

I'll merge it now but if you have time can you check if Claude has a better idea?
The problem is that in openms we have multiple modules that are compiled and ideally the ArrayWrapper or autowrap internals would be a separate module that others could import. But maybe that's also overkill. We should make an issue for that.