Skip to content

fix(cpp): resync headers, sources and bindings so the extension builds - #3306

Open
LuisFigueroaG wants to merge 1 commit into
im-anishraj:mainfrom
LuisFigueroaG:fix/frame-header-declarations
Open

LuisFigueroaG wants to merge 1 commit into
im-anishraj:mainfrom
LuisFigueroaG:fix/frame-header-declarations

Conversation

@LuisFigueroaG

Copy link
Copy Markdown
Contributor

Summary

Several merges left headers, sources and bindings out of sync, so the native extension stopped compiling on main and every CI job that builds it fails. This puts them back in sync with the smallest changes I could make:

  • frame.h: declare memory_usage(bool deep = false) and describe() as defined in frame.cpp, and restore the Frame(size_t row_count) constructor that the bindings and cpp/tests/test_cleaning.cpp still use
  • cleaning.h: declare cast_types as cleaning.cpp defines it (CastResult with a CastErrors policy) instead of two Frame overloads that were never implemented
  • csv_reader.cpp: restore the CsvParser::is_null_sentinel definition (declaration and callers were kept, definition was dropped)
  • bind_arnio.cpp: remove the stray } that ended PYBIND11_MODULE early, drop the create_rolling_windows binding (no C++ implementation left), and bind cast_types through the errors="raise"|"coerce"|"report" wrapper returning (frame, failures), which is what cleaning.py calls

Linked issue

Fixes #3305

Type of change

  • Bug fix

Area

  • C++ cleaning engine
  • CSV parser / I/O

Testing

Python 3.12, cmake 4.4, ninja, macOS (clang):

  • Full CMake build with -DARNIO_BUILD_CPP_TESTS=ON (core, extension, benchmark, all test binaries): no errors
  • pip install -e .: builds and installs; Frame(3), memory_usage(deep=True) and the three cast_types modes work when loading the extension directly
  • ctest: test_column, test_cleaning and test_csv_parser pass; test_frame passes 13/14. The failing case is "add_column throws on duplicate column name". That duplicate-name check was dropped in an earlier merge, and the failure was hidden on main because nothing compiled. It's a behaviour bug, so I left it out of this PR.
  • ASAN build with the flags from asan.yml: builds, no sanitizer reports, same single test_frame failure
  • git clang-format --diff upstream/main: nothing to change on the edited lines
  • make test: can't run yet. import arnio still fails on main for Python-side reasons outside this PR (cleaning.py is fixed in fix: dedupe cleaning.py and restore helpers dropped by bad merge #3304; frame.py, io.py, quality.py, schema.py and convert.py have similar merge damage)

Contributor checklist

  • I kept the PR focused on one issue or one logical change.
  • I checked that generated files, logs, and local build artifacts are not committed.
  • My PR title uses Conventional Commits.

Maintainer notes

Follow-ups I noticed but kept out of scope:

  • Some bindings no longer match what the Python layer calls (Frame.describe binding, from_dict(..., row_count), skip_rows/decimal_separator in the CSV config). They compile, but will raise once the Python side imports again.
  • rolling_window is still exported in __all__ and tested in tests/test_rolling_window.py, but has no implementation anymore.
  • Frame(std::vector<Column>, size_t) doesn't set row_count_known_.

🤖 Generated with Claude Code

Several merges left declarations, definitions and bindings out of sync,
so the native extension no longer compiled on main.

- frame.h: restore memory_usage(bool deep) and describe() declarations
  that match frame.cpp, plus the Frame(size_t row_count) constructor
  used by the bindings and the cleaning tests.
- cleaning.h: declare cast_types as it is defined in cleaning.cpp
  (CastResult with a CastErrors policy) instead of two Frame overloads
  that were never implemented.
- csv_reader.cpp: restore the CsvParser::is_null_sentinel definition,
  which was dropped while its declaration and callers were kept.
- bind_arnio.cpp: remove the stray closing brace that ended the module
  early, drop the create_rolling_windows binding (no native
  implementation exists anymore), and bind cast_types through the
  errors="raise"|"coerce"|"report" wrapper again.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 5, 2026

Copy link
Copy Markdown

@LuisFigueroaG is attempting to deploy a commit to the xtylishanish-gmailcom's projects Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

No deployments
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.

Bug: C++ extension fails to build on main (frame.h out of sync with frame.cpp and bindings)

1 participant