Skip to content

Use pybind for FortIO#1184

Open
eivindjahren wants to merge 9 commits into
mainfrom
fix_fortio
Open

Use pybind for FortIO#1184
eivindjahren wants to merge 9 commits into
mainfrom
fix_fortio

Conversation

@eivindjahren

@eivindjahren eivindjahren commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@eivindjahren eivindjahren requested a review from Copilot June 9, 2026 10:09
@eivindjahren eivindjahren self-assigned this Jun 9, 2026
@eivindjahren eivindjahren moved this to Ready for Review in SCOUT Jun 9, 2026
@eivindjahren eivindjahren force-pushed the fix_fortio branch 2 times, most recently from 4efb195 to 9521987 Compare June 9, 2026 10:11

Copilot AI left a comment

Copy link
Copy Markdown

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 FortIO integration to address FortIO-related failures by switching the Python FortIO wrapper from ResdataPrototype/cwrap-based symbol binding to a new pybind11 extension (resdata.resfile._fortio), and adds additional FortIO tests in both Python and C++.

Changes:

  • Add a pybind11 module (resdata.resfile._fortio) and update python/resdata/resfile/fortio.py to use it for open/seek/close/truncate/filename/ftell operations.
  • Modify FortIO C++ behavior (notably fortio_fseek, fortio_data_fseek, and fortio_fread_buffer) and add C++ tests covering these behaviors.
  • Add Python tests ensuring malformed Fortran records cause ResdataKW.fread() to return None.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/rd_tests/test_rd_kw.py Adds Python regression tests for malformed FortIO record handling.
python/resdata/resfile/fortio.py Switches Python FortIO wrapper to call into the new _fortio pybind module.
lib/tests/test_fortio.cpp Adds Catch2 tests for FortIO open/seek and error cases.
lib/resdata/tests/rd_fortio.cpp Removes instance/safe-cast tests from legacy FortIO tests.
lib/resdata/FortIO.cpp Refactors FortIO C/C++ implementation behavior and error handling.
lib/resdata/fortio_pybind.cpp Introduces pybind11 bridge between Python FortIO and C++ FortIO implementation.
lib/include/resdata/FortIO.hpp Converts header guards to #pragma once and adjusts exported declarations.
lib/CMakeLists.txt Builds/installs the new _fortio pybind11 extension and adds FortIO tests to the suite.

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

Comment thread lib/resdata/FortIO.cpp
Comment thread lib/include/resdata/FortIO.hpp
Comment thread lib/resdata/fortio_pybind.cpp
Comment thread lib/resdata/fortio_pybind.cpp
Comment thread python/resdata/resfile/fortio.py
Comment thread lib/include/resdata/FortIO.hpp
Comment thread lib/resdata/FortIO.cpp

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Comment thread lib/resdata/FortIO.cpp
Comment thread python/resdata/resfile/fortio.py
Comment thread lib/include/resdata/FortIO.hpp
Comment thread lib/include/resdata/FortIO.hpp
@eivindjahren eivindjahren force-pushed the fix_fortio branch 4 times, most recently from a1d1962 to 226f48a Compare June 9, 2026 11:44
@eivindjahren eivindjahren changed the title Fix fortio Use pybind for FortIO and fix some input validation Jun 9, 2026
@eivindjahren eivindjahren force-pushed the fix_fortio branch 3 times, most recently from b6d9e28 to b368e55 Compare June 10, 2026 07:11
@eivindjahren eivindjahren changed the title Use pybind for FortIO and fix some input validation Use pybind for FortIO Jun 10, 2026
The added logic_error should not be reachable.

Note that we keep the somewhat out of place exception
in fseek as opposed to returning false to not change to
a silent failure on the python side.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

2 participants