From 24ef2f54de48acab304e6b9a2296c893bd20ba8a Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Mon, 25 Aug 2025 16:24:30 +0200 Subject: [PATCH 01/49] Remove placeholder code --- meson.build | 1 - src/example_fgen_basic/operations.py | 35 ---------------------------- tests/unit/test_operations.py | 12 ---------- 3 files changed, 48 deletions(-) delete mode 100644 src/example_fgen_basic/operations.py delete mode 100644 tests/unit/test_operations.py diff --git a/meson.build b/meson.build index 93437af..97a3365 100644 --- a/meson.build +++ b/meson.build @@ -64,7 +64,6 @@ if pyprojectwheelbuild_enabled 'src/example_fgen_basic/__init__.py', 'src/example_fgen_basic/exceptions.py', 'src/example_fgen_basic/get_wavelength.py', - 'src/example_fgen_basic/operations.py', 'src/example_fgen_basic/runtime_helpers.py', ) diff --git a/src/example_fgen_basic/operations.py b/src/example_fgen_basic/operations.py deleted file mode 100644 index 84d8562..0000000 --- a/src/example_fgen_basic/operations.py +++ /dev/null @@ -1,35 +0,0 @@ -""" -Operations - -This module is just there to help with doc building etc. on project creation. -You will probably delete it early in the project. -""" - -from __future__ import annotations - - -def add_two(a: float, b: float) -> float: - """ - Add two numbers - - Parameters - ---------- - a - First number - - b - Second number - - Returns - ------- - : - Sum of the numbers - - Examples - -------- - >>> add_two(1, 3) - 4 - >>> add_two(1, -3) - -2 - """ - return a + b diff --git a/tests/unit/test_operations.py b/tests/unit/test_operations.py deleted file mode 100644 index 1f7fcf1..0000000 --- a/tests/unit/test_operations.py +++ /dev/null @@ -1,12 +0,0 @@ -""" -Test operations - -This file is just there to help demonstrate the initial setup. -You will probably delete it early in the project. -""" - -from example_fgen_basic.operations import add_two - - -def test_add_two(): - assert add_two(3.3, 4.4) == 7.7 From cedd97f48a90f98c5bb87c447024fb823cafa12a Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Mon, 25 Aug 2025 16:25:43 +0200 Subject: [PATCH 02/49] Update test header --- tests/unit/test_get_wavelength.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_get_wavelength.py b/tests/unit/test_get_wavelength.py index c75ac87..ed5c3db 100644 --- a/tests/unit/test_get_wavelength.py +++ b/tests/unit/test_get_wavelength.py @@ -1,5 +1,5 @@ """ -Dummy tests +Tests of `example_fgen_basic.get_wavelength` """ import numpy as np From 03e1b4e7c78cdfd5a31320d96f57224093e3c6bb Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Mon, 25 Aug 2025 16:35:05 +0200 Subject: [PATCH 03/49] Add failing test --- tests/unit/test_error_v_creation.py | 47 +++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 tests/unit/test_error_v_creation.py diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py new file mode 100644 index 0000000..72ae5a6 --- /dev/null +++ b/tests/unit/test_error_v_creation.py @@ -0,0 +1,47 @@ +""" +Tests of `example_fgen_basic.error_v.creation` + +Note that this is the only test of the Fortran code. +I haven't written unit tests for the Fortran directly +(deliberately, just to see how it goes). +""" + +from example_fgen_basic.error_v import ErrorV +from example_fgen_basic.error_v.creation import create_error + + +def test_create_error_odd(): + res = create_error(1.0) + + assert isinstance(res, ErrorV) + + assert res.code == 0 + assert res.message == "" + + +def test_create_error_even(): + res = create_error(2.0) + + assert isinstance(res, ErrorV) + + assert res.code != 0 + assert res.code == 1 + assert res.message == "Even number supplied" + + +def test_create_error_negative(): + res = create_error(-1.0) + + assert isinstance(res, ErrorV) + + assert res.code == 2 + assert res.message == "Negative number supplied" + + +# Tests to write: +# - if we create more errors than we have available, we don't segfault. +# Instead, we should get an error back. +# That error should just use the instance ID of the last available array index +# (it is ok to overwrite an already used error to avoid a complete failure, +# but we should probably include that we did this in the error message). +# - we can resize the available number of error instances to avoid hitting limits From 87cea35affb00582f9465accd29b644b74a07ca8 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Mon, 25 Aug 2025 16:48:30 +0200 Subject: [PATCH 04/49] Add Fortran back-end --- meson.build | 3 + src/example_fgen_basic/error_v/creation.f90 | 44 +++++++++ src/example_fgen_basic/error_v/error_v.f90 | 98 +++++++++++++++++++ .../fpyfgen/base_finalisable.f90 | 46 +++++++++ 4 files changed, 191 insertions(+) create mode 100644 src/example_fgen_basic/error_v/creation.f90 create mode 100644 src/example_fgen_basic/error_v/error_v.f90 create mode 100644 src/example_fgen_basic/fpyfgen/base_finalisable.f90 diff --git a/meson.build b/meson.build index 97a3365..03f4965 100644 --- a/meson.build +++ b/meson.build @@ -54,6 +54,9 @@ if pyprojectwheelbuild_enabled # Specify all the other source Fortran files (original files and managers) # Injected with `script/inject-srcs-into-meson-build.py` srcs_ancillary_lib = files( + 'src/example_fgen_basic/error_v/creation.f90', + 'src/example_fgen_basic/error_v/error_v.f90', + 'src/example_fgen_basic/fpyfgen/base_finalisable.f90', 'src/example_fgen_basic/get_wavelength.f90', 'src/example_fgen_basic/kind_parameters.f90', ) diff --git a/src/example_fgen_basic/error_v/creation.f90 b/src/example_fgen_basic/error_v/creation.f90 new file mode 100644 index 0000000..3ad7cd8 --- /dev/null +++ b/src/example_fgen_basic/error_v/creation.f90 @@ -0,0 +1,44 @@ +!> Error creation +!> +!> A very basic demo to get the idea. +! +! TODO: discuss - we should probably have some convention for module names. +! The hard part is avoiding them becoming too long... +module m_error_v_creation + + use m_error_v, only: ErrorV, NO_ERROR_CODE + + implicit none + private + + public :: create_error + +contains + + function create_error(inv) result(err) + !! Create an error + !! + !! If an odd number is supplied, the error code is no error (TODO: cross-ref). + !! If an even number is supplied, the error code is 1. + !! If a negative number is supplied, the error code is 2. + + integer, intent(in) :: inv + !! Value to use to create the error + + type(ErrorV) :: err + !! Created error + + if (inv < 0) then + err = ErrorV(code=2, message="Negative number supplied") + return + end if + + if (mod(inv, 2) .eq. 0) then + err = ErrorV(code=1, message="Even number supplied") + else + err = ErrorV(code=0) + end if + + end function create_error + +end module m_error_v_creation diff --git a/src/example_fgen_basic/error_v/error_v.f90 b/src/example_fgen_basic/error_v/error_v.f90 new file mode 100644 index 0000000..166be59 --- /dev/null +++ b/src/example_fgen_basic/error_v/error_v.f90 @@ -0,0 +1,98 @@ +!> Error value +!> +!> Inspired by the excellent, MIT licensed +!> https://github.com/samharrison7/fortran-error-handler +!> +!> Fortran doesn't have a null value. +!> As a result, we introduce this derived type +!> with the convention that a code of 0 indicates no error. +module m_error_v + + ! TODO: switch `m_` prefix to project name prefix + ! (e.g. fpyfgen, "Fortran-code for Python-Fortran wrapper Generator") + ! (same idea as `tomlf_` prefix) + ! TODO: move this to standalone repo (eventually) + use fpyfgen_base_finalisable, only: BaseFinalisable + + implicit none + private + + integer, parameter, public :: NO_ERROR_CODE = 0 + !! Code that indicates no error + + type, extends(BaseFinalisable), public :: ErrorV + !! Error value + + integer :: code = 1 + !! Error code + + character(len=128) :: message = "" + ! TODO: think about making the message allocatable to handle long messages + + ! TODO: think about adding idea of critical + ! (means you can stop but also unwind errors and traceback along the way) + + ! TODO: think about adding trace (might be simpler than compiling with traceback) + + contains + + private + + procedure, public:: build, finalise + ! get_res sort of not needed (?) + ! get_err sort of not needed (?) + + end type ErrorV + + interface ErrorV + !! Constructor interface - see build (TODO: figure out cross-ref syntax) for details + ! This is what allows us to do ErrorV(...) when using this class + module procedure :: constructor + end interface ErrorV + +contains + + function constructor(code, message) result(self) + !! Constructor - see build (TODO: figure out cross-ref syntax) for details + + integer, intent(in) :: code + character(len=*), optional, intent(in) :: message + + type(ErrorV) :: self + + call self % build(code, message) + + end function constructor + + subroutine build(self, code, message) + !! Build instance + + class(ErrorV), intent(inout) :: self + ! Hopefully can leave without docstring (like Python) + + integer, intent(in) :: code + !! Error code + !! + !! Use [TODO: figure out xref] `NO_ERROR_CODE` if there is no error + + character(len=*), optional, intent(in) :: message + !! Error message + + self % code = code + if (present(message)) then + self % message = message + end if + + end subroutine build + + subroutine finalise(self) + !! Finalise the instance (i.e. free/deallocate) + + class(ErrorV), intent(inout) :: self + ! Hopefully can leave without docstring (like Python) + + ! If we make message allocatable, deallocate here + + end subroutine finalise + +end module m_error_v diff --git a/src/example_fgen_basic/fpyfgen/base_finalisable.f90 b/src/example_fgen_basic/fpyfgen/base_finalisable.f90 new file mode 100644 index 0000000..a55df95 --- /dev/null +++ b/src/example_fgen_basic/fpyfgen/base_finalisable.f90 @@ -0,0 +1,46 @@ +!> Base class for classes that can be wrapped with pyfgen. +!> +!> Such classes must always be finalisable, to help with memory management +!> across the Python-Fortran interface. +module fpyfgen_base_finalisable + + implicit none + private + + integer, parameter, public :: INVALID_INSTANCE_INDEX = -1 + !! Value that denotes an invalid model index + + public :: BaseFinalisable + + type, abstract :: BaseFinalisable + + integer :: instance_index = INVALID_INSTANCE_INDEX + !! Unique identifier for the instance. + !! + !! Set to a value > 0 when the instance is in use, + !! set to `INVALID_INSTANCE_INDEX` (TODO xref) otherwise. + !! The value is linked to the position in a manager array stored elsewhere. + !! This value shouldn't be modified from outside the manager + !! unless you really know what you're doing. + + contains + + private + + procedure(derived_type_finalise), public, deferred :: finalise + + end type BaseFinalisable + + interface + + subroutine derived_type_finalise(self) + !! Finalise the instance (i.e. free/deallocate) + + import :: BaseFinalisable + class(BaseFinalisable), intent(inout) :: self + + end subroutine derived_type_finalise + + end interface + +end module fpyfgen_base_finalisable From fdf16358e92bf89710ef93d28ae84f4dc0477fe8 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Mon, 25 Aug 2025 16:57:05 +0200 Subject: [PATCH 05/49] Add in manager code --- meson.build | 3 + .../error_v/creation_wrapper.f90 | 92 +++++++++++++++ .../error_v/error_v_manager.f90 | 108 ++++++++++++++++++ .../fpyfgen/derived_type_manager_helpers.f90 | 77 +++++++++++++ 4 files changed, 280 insertions(+) create mode 100644 src/example_fgen_basic/error_v/creation_wrapper.f90 create mode 100644 src/example_fgen_basic/error_v/error_v_manager.f90 create mode 100644 src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 diff --git a/meson.build b/meson.build index 03f4965..8642b0d 100644 --- a/meson.build +++ b/meson.build @@ -48,6 +48,7 @@ if pyprojectwheelbuild_enabled # These are the src with which Python interacts. # Injected with `script/inject-srcs-into-meson-build.py` srcs = files( + 'src/example_fgen_basic/error_v/creation_wrapper.f90', 'src/example_fgen_basic/get_wavelength_wrapper.f90', ) @@ -56,7 +57,9 @@ if pyprojectwheelbuild_enabled srcs_ancillary_lib = files( 'src/example_fgen_basic/error_v/creation.f90', 'src/example_fgen_basic/error_v/error_v.f90', + 'src/example_fgen_basic/error_v/error_v_manager.f90', 'src/example_fgen_basic/fpyfgen/base_finalisable.f90', + 'src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90', 'src/example_fgen_basic/get_wavelength.f90', 'src/example_fgen_basic/kind_parameters.f90', ) diff --git a/src/example_fgen_basic/error_v/creation_wrapper.f90 b/src/example_fgen_basic/error_v/creation_wrapper.f90 new file mode 100644 index 0000000..417a91c --- /dev/null +++ b/src/example_fgen_basic/error_v/creation_wrapper.f90 @@ -0,0 +1,92 @@ +!> Wrapper for interfacing `m_error_v_creation` with Python +!> +!> Written by hand here. +!> Generation to be automated in future (including docstrings of some sort). +module m_error_v_creation_w + + ! => allows us to rename on import to avoid clashes + use m_error_v_creation, only: o_create_error => create_error + use m_error_v, only: ErrorV + + ! The manager module, which makes this all work + use m_error_v_manager, only: & + error_v_manager_get_free_instance_number => get_free_instance_number, & + error_v_manager_associate_pointer_with_instance => associate_pointer_with_instance + ! TODO: finalisation + + implicit none + private + + public :: create_error, iget_code, iget_message + +contains + + subroutine create_error(inv, res_instance_index) + ! Needs to be subroutine to have the created instance persist I think + ! (we can check) + ! function create_error(inv) result(res_instance_index) + + integer, intent(in) :: inv + !! Input value to use to create the error + + integer, intent(out) :: res_instance_index + !! Instance index of the result + ! + ! This is the major trick for wrapping. + ! We return instance indexes (integers) to Python rather than the instance itself. + + type(ErrorV), pointer :: res + + ! This is the other trick for wrapping. + ! We have to ensure that we have correctly associated pointers + ! with the derived type instances we want to 'pass' across the Python-Fortran interface. + ! Once we've done this, we can then set them more or less like normal derived types. + res_instance_index = error_v_manager_get_free_instance_number() + call error_v_manager_associate_pointer_with_instance(res_instance_index, res) + + ! Use the pointer more or less like a normal instance of the derived type + res = o_create_error(inv) + ! Ensure that the instance index is set correctly + res % instance_index = res_instance_index + + end subroutine create_error + + ! Full set of wrapping strategies to pass different types in e.g. + ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 + ! (we will do a full re-write of the code which generates this, + ! but the strategies will probably stay as they are) + subroutine iget_code( & + instance_index, & + code & + ) + + integer, intent(in) :: instance_index + + integer, intent(out) :: code + + type(ErrorV), pointer :: instance + + call error_v_manager_associate_pointer_with_instance(instance_index, instance) + + code = instance % code + + end subroutine iget_code + + subroutine iget_message( & + instance_index, & + message & + ) + + integer, intent(in) :: instance_index + + character(len=128), intent(out) :: message + + type(ErrorV), pointer :: instance + + call error_v_manager_associate_pointer_with_instance(instance_index, instance) + + message = instance % message + + end subroutine iget_message + +end module m_error_v_creation_w diff --git a/src/example_fgen_basic/error_v/error_v_manager.f90 b/src/example_fgen_basic/error_v/error_v_manager.f90 new file mode 100644 index 0000000..6292c4a --- /dev/null +++ b/src/example_fgen_basic/error_v/error_v_manager.f90 @@ -0,0 +1,108 @@ +!> Manager of `ErrorV` (TODO: xref) across the Fortran-Python interface +!> +!> Written by hand here. +!> Generation to be automated in future (including docstrings of some sort). +! +! TODO: make it possible to reallocate the number of instances +module m_error_v_manager + + use fpyfgen_derived_type_manager_helpers, only: finalise_derived_type_instance_number, & + get_derived_type_free_instance_number + use m_error_v, only: ErrorV + + implicit none + private + + integer, public, parameter :: N_INSTANCES_DEFAULT = 4096 + !! Default maximum number of instances which can be created simultaneously + ! + ! TODO: allow reallocation if possible + + ! This is the other trick, we hold an array of instances + ! for tracking what is being passed back and forth across the interface. + type(ErrorV), target, dimension(N_INSTANCES_DEFAULT) :: instance_array + logical, dimension(N_INSTANCES_DEFAULT) :: instance_available = .true. + + public :: get_free_instance_number, & + associate_pointer_with_instance, & + finalise_instance + +contains + + function get_free_instance_number() result(instance_index) + !! Get the index of a free instance + + integer :: instance_index + !! Free instance index + + call get_derived_type_free_instance_number( & + instance_index, & + N_INSTANCES_DEFAULT, & + instance_available, & + instance_array & + ) + + end function get_free_instance_number + + subroutine associate_pointer_with_instance(instance_index, instance_pointer) + !! Associate a pointer with the instance corresponding to the given model index + !! + !! Stops execution if the instance has not already been initialised. + + integer, intent(in) :: instance_index + !! Index of the instance to point to + + type(ErrorV), pointer, intent(inout) :: instance_pointer + !! Pointer to associate + + call check_index_claimed(instance_index) + instance_pointer => instance_array(instance_index) + + end subroutine associate_pointer_with_instance + + subroutine finalise_instance(instance_index) + !! Finalise an instance + + integer, intent(in) :: instance_index + !! Index of the instance to finalise + + call check_index_claimed(instance_index) + call finalise_derived_type_instance_number( & + instance_index, & + N_INSTANCES_DEFAULT, & + instance_available, & + instance_array & + ) + + end subroutine finalise_instance + + subroutine check_index_claimed(instance_index) + !! Check that an index has already been claimed + !! + !! Stops execution if the index has not been claimed. + + integer, intent(in) :: instance_index + !! Instance index to check + + if (instance_available(instance_index)) then + ! TODO: switch to errors here - will require some thinking + print *, "Index ", instance_index, " has not been claimed" + error stop 1 + end if + + if (instance_index < 1) then + ! TODO: switch to errors here - will require some thinking + print *, "Requested index is ", instance_index, " which is less than 1" + error stop 1 + end if + + if (instance_array(instance_index) % instance_index < 1) then + ! TODO: switch to errors here - will require some thinking + print *, "Index ", instance_index, " is associated with an instance that has instance index < 1", & + "instance's instance_index attribute ", instance_array(instance_index) % instance_index + error stop 1 + end if + + end subroutine check_index_claimed + +end module m_error_v_manager diff --git a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 new file mode 100644 index 0000000..92cd3d6 --- /dev/null +++ b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 @@ -0,0 +1,77 @@ +!> Helpers for derived type managers +module fpyfgen_derived_type_manager_helpers + + use fpyfgen_base_finalisable, only: BaseFinalisable, invalid_instance_index + + implicit none + private + + public :: get_derived_type_free_instance_number, & + finalise_derived_type_instance_number + +contains + + subroutine get_derived_type_free_instance_number(instance_index, n_instances, instance_avail, instance_array) + !! Get the next available instance number + !! + !! If successful, `instance_index` will contain a positive value. + !! If no available instances are found, + !! instance_index will be set to `invalid_instance_index`. + !! TODO: change the above to return a Result type instead + + integer, intent(out) :: instance_index + !! Free index + !! + !! If no available instances are found, set to `invalid_instance_index`. + + integer, intent(in) :: n_instances + !! Size of `instance_avail` + + logical, dimension(n_instances), intent(inout) :: instance_avail + !! Array that indicates whether each index is available or not + + class(BaseFinalisable), dimension(n_instances), intent(inout) :: instance_array + !! Array of instances + + integer :: i + + ! Default if no available models are found + instance_index = invalid_instance_index + + do i = 1, n_instances + + if (instance_avail(i)) then + + instance_avail(i) = .false. + instance_array(i) % instance_index = i + instance_index = i + return + + end if + + end do + + end subroutine get_derived_type_free_instance_number + + subroutine finalise_derived_type_instance_number(instance_index, n_instances, instance_avail, instance_array) + !! Finalise the derived type with the given instance index + + integer, intent(in) :: instance_index + !! Index of the instance to finalise + + integer, intent(in) :: n_instances + !! Size of `instance_avail` + + logical, dimension(n_instances), intent(inout) :: instance_avail + !! Array that indicates whether each index is available or not + + class(BaseFinalisable), dimension(n_instances), intent(inout) :: instance_array + !! Array of instances + + call instance_array(instance_index) % finalise() + instance_array(instance_index) % instance_index = invalid_instance_index + instance_avail(instance_index) = .true. + + end subroutine finalise_derived_type_instance_number + +end module fpyfgen_derived_type_manager_helpers From 331c58690a00cff8b26a936f0f39828eeb84fca1 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Mon, 25 Aug 2025 17:32:03 +0200 Subject: [PATCH 06/49] Trying to figure out how to get python file install path right --- meson.build | 15 +- src/example_fgen_basic/error_v/__init__.py | 7 + src/example_fgen_basic/error_v/creation.py | 26 ++ src/example_fgen_basic/error_v/error_v.py | 68 +++++ src/example_fgen_basic/get_wavelength.py | 4 +- .../pyfgen_runtime/__init__.py | 7 + .../pyfgen_runtime/base_finalisable.py | 165 ++++++++++++ .../pyfgen_runtime/exceptions.py | 80 ++++++ .../pyfgen_runtime/formatting.py | 234 ++++++++++++++++++ 9 files changed, 604 insertions(+), 2 deletions(-) create mode 100644 src/example_fgen_basic/error_v/__init__.py create mode 100644 src/example_fgen_basic/error_v/creation.py create mode 100644 src/example_fgen_basic/error_v/error_v.py create mode 100644 src/example_fgen_basic/pyfgen_runtime/__init__.py create mode 100644 src/example_fgen_basic/pyfgen_runtime/base_finalisable.py create mode 100644 src/example_fgen_basic/pyfgen_runtime/exceptions.py create mode 100644 src/example_fgen_basic/pyfgen_runtime/formatting.py diff --git a/meson.build b/meson.build index 8642b0d..ca0f527 100644 --- a/meson.build +++ b/meson.build @@ -12,6 +12,9 @@ project( ], ) +# https://mesonbuild.com/Fs-module.html +fs = import('fs') + # Some useful constants pyprojectwheelbuild_enabled = get_option('pyprojectwheelbuild').enabled() @@ -68,8 +71,15 @@ if pyprojectwheelbuild_enabled # Injected with `script/inject-srcs-into-meson-build.py` python_srcs = files( 'src/example_fgen_basic/__init__.py', + 'src/example_fgen_basic/error_v/__init__.py', + 'src/example_fgen_basic/error_v/creation.py', + 'src/example_fgen_basic/error_v/error_v.py', 'src/example_fgen_basic/exceptions.py', 'src/example_fgen_basic/get_wavelength.py', + 'src/example_fgen_basic/pyfgen_runtime/__init__.py', + 'src/example_fgen_basic/pyfgen_runtime/base_finalisable.py', + 'src/example_fgen_basic/pyfgen_runtime/exceptions.py', + 'src/example_fgen_basic/pyfgen_runtime/formatting.py', 'src/example_fgen_basic/runtime_helpers.py', ) @@ -137,9 +147,12 @@ if pyprojectwheelbuild_enabled # this is where that operation happens. # Files get copied to /site-packages/ foreach python_src : python_srcs + message(python_src) + message(python_project_name) + message(fs.relative_to(python_src, python_project_name)) py.install_sources( python_src, - subdir: python_project_name, + subdir: python_project_name / fs.relative_to(python_src, python_project_name), pure: false, ) diff --git a/src/example_fgen_basic/error_v/__init__.py b/src/example_fgen_basic/error_v/__init__.py new file mode 100644 index 0000000..4182384 --- /dev/null +++ b/src/example_fgen_basic/error_v/__init__.py @@ -0,0 +1,7 @@ +""" +Definition of an error value +""" + +from example_fgen_basic.error_v.error_v import ErrorV + +__all__ = ["ErrorV"] diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py new file mode 100644 index 0000000..5c0dce0 --- /dev/null +++ b/src/example_fgen_basic/error_v/creation.py @@ -0,0 +1,26 @@ +""" +Demonstration of how to return an error (i.e. derived type) +""" + +from __future__ import annotations + +from example_fgen_basic.error_v.error_v import ErrorV +from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError + +try: + from example._lib import ( # type: ignore + m_error_v_creation_w, + ) +except (ModuleNotFoundError, ImportError) as exc: + raise CompiledExtensionNotFoundError("example._lib.m_error_v_creation_w") from exc + + +def create_error(inv: int) -> ErrorV: + """ + Create an instance of error (a wrapper around our Fortran derived type) + """ + instance_index = m_error_v_creation_w.create_error(inv) + + error = ErrorV(instance_index) + + return error diff --git a/src/example_fgen_basic/error_v/error_v.py b/src/example_fgen_basic/error_v/error_v.py new file mode 100644 index 0000000..7b637d4 --- /dev/null +++ b/src/example_fgen_basic/error_v/error_v.py @@ -0,0 +1,68 @@ +""" +Wrapper of the Fortran :class:`ErrorV` +""" + +from __future__ import annotations + +from attrs import define + +from example_fgen_basic.pyfgen_runtime.base_finalisable import ( + FinalisableWrapperBase, + check_initialised, +) +from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError + +try: + from example._lib import ( # type: ignore + m_error_v_creation_w, + ) +except (ModuleNotFoundError, ImportError) as exc: + raise CompiledExtensionNotFoundError("example._lib.m_error_v_creation_w") from exc + + +@define +class ErrorV(FinalisableWrapperBase): + """ + TODO: auto docstring e.g. "Wrapper around the Fortran :class:`ErrorV`" + """ + + @property + def exposed_attributes(self) -> tuple[str, ...]: + """ + Attributes exposed by this wrapper + """ + return ("code", "message") + + # TODO: from_build_args, from_new_connection, context manager, finalise + + @property + @check_initialised + def code(self) -> int: + """ + Error code + + Returns + ------- + : + Error code, retrieved from Fortran + """ + code: int = m_error_v_creation_w.iget_code(instance_index=self.instance_index) + + return code + + @property + @check_initialised + def message(self) -> str: + """ + Error message + + Returns + ------- + : + Error message, retrieved from Fortran + """ + message: str = m_error_v_creation_w.iget_message( + instance_index=self.instance_index + ).decode() + + return message diff --git a/src/example_fgen_basic/get_wavelength.py b/src/example_fgen_basic/get_wavelength.py index c6127a6..c00c4b7 100644 --- a/src/example_fgen_basic/get_wavelength.py +++ b/src/example_fgen_basic/get_wavelength.py @@ -18,7 +18,9 @@ try: from example_fgen_basic._lib import m_get_wavelength_w # type: ignore except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover - raise CompiledExtensionNotFoundError("example_fgen_basic._lib") from exc + raise CompiledExtensionNotFoundError( + "example_fgen_basic._lib.m_get_wavelength_w" + ) from exc def get_wavelength_plain(frequency: float) -> float: diff --git a/src/example_fgen_basic/pyfgen_runtime/__init__.py b/src/example_fgen_basic/pyfgen_runtime/__init__.py new file mode 100644 index 0000000..d9e9108 --- /dev/null +++ b/src/example_fgen_basic/pyfgen_runtime/__init__.py @@ -0,0 +1,7 @@ +""" +Runtime helpers for code generated with pyfgen + +Code that will eventually get moved into a standalone package +""" + +from __future__ import annotations diff --git a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py new file mode 100644 index 0000000..dd3e80a --- /dev/null +++ b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py @@ -0,0 +1,165 @@ +""" +Runtime helper to support wrapping of Fortran derived types +""" + +from __future__ import annotations + +from abc import ABC, abstractmethod +from functools import wraps +from typing import Any, Callable, Concatenate, ParamSpec, TypeVar + +import attrs +from attrs import define, field + +from example_fgen_basic.pyfgen_runtime.exceptions import ( + NotInitialisedError, +) +from example_fgen_basic.pyfgen_runtime.formatting import to_html, to_pretty, to_str + +# Might be needed for Python 3.9 +# from typing_extensions import Concatenate, ParamSpec + + +INVALID_INSTANCE_INDEX: int = -1 +""" +Value used to denote an invalid `instance_index`. + +This can occur value when a wrapper class +has not yet been initialised (connected to a Fortran instance). +""" + + +@define +class FinalisableWrapperBase(ABC): + """ + Base class for Fortran derived type wrappers + """ + + instance_index: int = field( + validator=attrs.validators.instance_of(int), + default=INVALID_INSTANCE_INDEX, + ) + """ + Model index of wrapper Fortran instance + """ + + def __str__(self) -> str: + """ + Get string representation of self + """ + return to_str( + self, + self.exposed_attributes, + ) + + def _repr_pretty_(self, p: Any, cycle: bool) -> None: + """ + Get pretty representation of self + + Used by IPython notebooks and other tools + """ + to_pretty( + self, + self.exposed_attributes, + p=p, + cycle=cycle, + ) + + def _repr_html_(self) -> str: + """ + Get html representation of self + + Used by IPython notebooks and other tools + """ + return to_html( + self, + self.exposed_attributes, + ) + + @property + def initialized(self) -> bool: + """ + Is the instance initialised, i.e. connected to a Fortran instance? + """ + return self.instance_index != INVALID_INSTANCE_INDEX + + @property + @abstractmethod + def exposed_attributes(self) -> tuple[str, ...]: + """ + Attributes exposed by this wrapper + """ + ... + + # TODO: consider whether we need these + # @classmethod + # @abstractmethod + # def from_new_connection(cls) -> FinalisableWrapperBase: + # """ + # Initialise by establishing a new connection with the Fortran module + # + # This requests a new model index from the Fortran module and then + # initialises a class instance + # + # Returns + # ------- + # New class instance + # """ + # ... + # + # @abstractmethod + # def finalize(self) -> None: + # """ + # Finalise the Fortran instance and set self back to being uninitialised + # + # This method resets ``self.instance_index`` back to + # ``_UNINITIALISED_instance_index`` + # + # Should be decorated with :func:`check_initialised` + # """ + # # call to Fortran module goes here when implementing + # self._uninitialise_instance_index() + + def _uninitialise_instance_index(self) -> None: + self.instance_index = INVALID_INSTANCE_INDEX + + +P = ParamSpec("P") +T = TypeVar("T") +Wrapper = TypeVar("Wrapper", bound=FinalisableWrapperBase) + + +def check_initialised( + method: Callable[Concatenate[Wrapper, P], T], +) -> Callable[Concatenate[Wrapper, P], T]: + """ + Check that the wrapper object has been initialised before executing the method + + Parameters + ---------- + method + Method to wrap + + Returns + ------- + : + Wrapped method + + Raises + ------ + InitialisationError + Wrapper is not initialised + """ + + @wraps(method) + def checked( + ref: Wrapper, + *args: P.args, + **kwargs: P.kwargs, + ) -> Any: + if not ref.initialized: + raise NotInitialisedError(ref, method) + + return method(ref, *args, **kwargs) + + return checked # type: ignore diff --git a/src/example_fgen_basic/pyfgen_runtime/exceptions.py b/src/example_fgen_basic/pyfgen_runtime/exceptions.py new file mode 100644 index 0000000..0dbf4f7 --- /dev/null +++ b/src/example_fgen_basic/pyfgen_runtime/exceptions.py @@ -0,0 +1,80 @@ +""" +Runtime exceptions +""" + +from __future__ import annotations + +from typing import Any, Callable, Optional + + +class CompiledExtensionNotFoundError(ImportError): + """ + Raised when a compiled extension can't be imported i.e. found + """ + + def __init__(self, compiled_extension_name: str): + error_msg = f"Could not find compiled extension {compiled_extension_name!r}" + + super().__init__(error_msg) + + +class MissingOptionalDependencyError(ImportError): + """ + Raised when an optional dependency is missing + + For example, plotting dependencies like matplotlib + """ + + def __init__(self, callable_name: str, requirement: str) -> None: + """ + Initialise the error + + Parameters + ---------- + callable_name + The name of the callable that requires the dependency + + requirement + The name of the requirement + """ + error_msg = f"`{callable_name}` requires {requirement} to be installed" + super().__init__(error_msg) + + +class WrapperError(ValueError): + """ + Base exception for errors that arise from wrapper functionality + """ + + +class NotInitialisedError(WrapperError): + """ + Raised when the wrapper around the Fortran module hasn't been initialised yet + """ + + def __init__(self, instance: Any, method: Optional[Callable[..., Any]] = None): + if method: + error_msg = f"{instance} must be initialised before {method} is called" + else: + error_msg = f"instance ({instance:r}) is not initialized yet" + + super().__init__(error_msg) + + +# TODO: change or even remove this when we move to better error handling +class UnallocatedMemoryError(ValueError): + """ + Raised when we try to access memory that has not yet been allocated + + We can't always catch this error, but this is what we raise when we can. + """ + + def __init__(self, variable_name: str): + error_msg = ( + f"The memory required to access `{variable_name}` is unallocated. " + "You must allocate it before trying to access its value. " + "Unfortunately, we cannot provide more information " + "about why this memory is not yet allocated." + ) + + super().__init__(error_msg) diff --git a/src/example_fgen_basic/pyfgen_runtime/formatting.py b/src/example_fgen_basic/pyfgen_runtime/formatting.py new file mode 100644 index 0000000..ecd4e61 --- /dev/null +++ b/src/example_fgen_basic/pyfgen_runtime/formatting.py @@ -0,0 +1,234 @@ +""" +Formatting support + +I.e. display of wrapped Fortran derived types in a nice way +""" + +from __future__ import annotations + +from collections.abc import Iterable +from typing import TYPE_CHECKING, Any + +from example_fgen_basic.pyfgen_runtime.exceptions import UnallocatedMemoryError + +# Might be needed for Python 3.9 +# from typing_extensions import Concatenate, ParamSpec +if TYPE_CHECKING: + from example_fgen_basic.pyfgen_runtime.base_finalisable import ( + FinalisableWrapperBase, + ) + + +def get_attribute_str_value(instance: FinalisableWrapperBase, attribute: str) -> str: + """ + Get the string version of an attribute's value + + Parameters + ---------- + instance + Instance from which to get the attribute + + attribute + Attribute for which to get the value + + Returns + ------- + String version of the attribute's value, with graceful handling of errors. + """ + try: + return f"{attribute}={getattr(instance, attribute)}" + except UnallocatedMemoryError: + # TODO: change this when we move to better error handling + return f"{attribute} is unallocated" + + +def to_str(instance: FinalisableWrapperBase, exposed_attributes: Iterable[str]) -> str: + """ + Convert an instance to its string representation + + Parameters + ---------- + instance + Instance to convert + + exposed_attributes + Attributes from Fortran that the instance exposes + + Returns + ------- + String representation of the instance + """ + if not instance.initialized: + return f"Uninitialised {instance!r}" + + if not exposed_attributes: + return repr(instance) + + attribute_values = [ + get_attribute_str_value(instance, v) for v in exposed_attributes + ] + + return f"{repr(instance)[:-1]}, {', '.join(attribute_values)})" + + +def to_pretty( + instance: FinalisableWrapperBase, + exposed_attributes: Iterable[str], + p: Any, + cycle: bool, + indent: int = 4, +) -> None: + """ + Pretty-print an instance + + Parameters + ---------- + instance + Instance to convert + + exposed_attributes + Attributes from Fortran that the instance exposes + + p + Pretty printing object + + cycle + Whether the pretty printer has detected a cycle or not. + + indent + Indent to apply to the pretty printing group + """ + if not instance.initialized: + p.text(str(instance)) + return + + if not exposed_attributes: + p.text(str(instance)) + return + + with p.group(indent, f"{repr(instance)[:-1]}", ")"): + for att in exposed_attributes: + p.text(",") + p.breakable() + + p.text(get_attribute_str_value(instance, att)) + + +def add_attribute_row( + attribute_name: str, attribute_value: str, attribute_rows: list[str] +) -> list[str]: + """ + Add a row for displaying an attribute's value to a list of rows + + Parameters + ---------- + attribute_name + Attribute's name + + attribute_value + Attribute's value + + attribute_rows + Existing attribute rows + + + Returns + ------- + Attribute rows, with the new row appended + """ + attribute_rows.append( + f"{attribute_name}{attribute_value}" # noqa: E501 + ) + + return attribute_rows + + +def to_html(instance: FinalisableWrapperBase, exposed_attributes: Iterable[str]) -> str: + """ + Convert an instance to its html representation + + Parameters + ---------- + instance + Instance to convert + + exposed_attributes + Attributes from Fortran that the instance exposes + + Returns + ------- + HTML representation of the instance + """ + if not instance.initialized: + return str(instance) + + if not exposed_attributes: + return str(instance) + + instance_class_name = repr(instance).split("(")[0] + + attribute_rows: list[str] = [] + for att in exposed_attributes: + try: + att_val = getattr(instance, att) + except UnallocatedMemoryError: + # TODO: change this when we move to better error handling + att_val = "Unallocated" + attribute_rows = add_attribute_row(att, att_val, attribute_rows) + continue + + try: + att_val = att_val._repr_html_() + except AttributeError: + att_val = str(att_val) + + attribute_rows = add_attribute_row(att, att_val, attribute_rows) + + attribute_rows_for_table = "\n ".join(attribute_rows) + + css_style = """.fgen-wrap { + /*font-family: monospace;*/ + width: 540px; +} + +.fgen-header { + padding: 6px 0 6px 3px; + border-bottom: solid 1px #777; + color: #555;; +} + +.fgen-header > div { + display: inline; + margin-top: 0; + margin-bottom: 0; +} + +.fgen-basefinalizable-cls, +.fgen-basefinalizable-instance-index { + margin-left: 2px; + margin-right: 10px; +} + +.fgen-basefinalizable-cls { + font-weight: bold; + color: #000000; +}""" + + return "\n".join( + [ + "
", + " ", + "
", + "
", + f"
{instance_class_name}
", + f"
instance_index={instance.instance_index}
", # noqa: E501 + " ", + f" {attribute_rows_for_table}", + "
", + "
", + "
", + "
", + ] + ) From 75d78bb9ce5711b67c176add96a9be943bf15a8a Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Mon, 25 Aug 2025 22:52:26 +0200 Subject: [PATCH 07/49] Add tests and compilation --- meson.build | 6 +-- pyproject.toml | 2 + requirements-only-tests-locked.txt | 23 ++++++++- requirements-only-tests-min-locked.txt | 3 ++ src/example_fgen_basic/error_v/creation.py | 4 +- src/example_fgen_basic/error_v/error_v.py | 15 +++++- tests/unit/test_error_v_creation.py | 47 ++++++++++++++++--- .../test_error_html.html | 41 ++++++++++++++++ .../test_error_pprint.txt | 1 + .../test_error_v_creation/test_error_str.txt | 1 + 10 files changed, 127 insertions(+), 16 deletions(-) create mode 100644 tests/unit/test_error_v_creation/test_error_html.html create mode 100644 tests/unit/test_error_v_creation/test_error_pprint.txt create mode 100644 tests/unit/test_error_v_creation/test_error_str.txt diff --git a/meson.build b/meson.build index ca0f527..7a32154 100644 --- a/meson.build +++ b/meson.build @@ -147,12 +147,10 @@ if pyprojectwheelbuild_enabled # this is where that operation happens. # Files get copied to /site-packages/ foreach python_src : python_srcs - message(python_src) - message(python_project_name) - message(fs.relative_to(python_src, python_project_name)) + message(fs.relative_to(python_src, 'src' / python_project_name)) py.install_sources( python_src, - subdir: python_project_name / fs.relative_to(python_src, python_project_name), + subdir: fs.parent(fs.relative_to(python_src, 'src')), pure: false, ) diff --git a/pyproject.toml b/pyproject.toml index 78510df..0fa52ab 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,6 +101,7 @@ tests-min = [ # Key dependencies # ---------------- "pytest>=8.3.4", + "pytest-regressions>=2.8.2", ] # Full test dependencies. tests-full = [ @@ -114,6 +115,7 @@ tests-full = [ tests = [ {include-group = "tests-min"}, {include-group = "tests-full"}, + "ipython>=8.18.1", ] all-dev = [ {include-group = "dev"}, diff --git a/requirements-only-tests-locked.txt b/requirements-only-tests-locked.txt index 58f28a8..07d8697 100644 --- a/requirements-only-tests-locked.txt +++ b/requirements-only-tests-locked.txt @@ -1,12 +1,33 @@ # This file was autogenerated by uv via the following command: # uv export -o requirements-only-tests-locked.txt --no-hashes --no-dev --no-emit-project --only-group tests +asttokens==3.0.0 colorama==0.4.6 ; sys_platform == 'win32' coverage==7.6.10 +decorator==5.1.1 exceptiongroup==1.3.0 ; python_full_version < '3.11' +executing==2.1.0 iniconfig==2.0.0 +ipython==8.18.1 ; python_full_version < '3.10' +ipython==8.37.0 ; python_full_version == '3.10.*' +ipython==9.4.0 ; python_full_version >= '3.11' +ipython-pygments-lexers==1.1.1 ; python_full_version >= '3.11' +jedi==0.19.2 +matplotlib-inline==0.1.7 packaging==24.2 +parso==0.8.4 +pexpect==4.9.0 ; (python_full_version < '3.10' and sys_platform == 'emscripten') or (sys_platform != 'emscripten' and sys_platform != 'win32') pluggy==1.5.0 +prompt-toolkit==3.0.48 +ptyprocess==0.7.0 ; (python_full_version < '3.10' and sys_platform == 'emscripten') or (sys_platform != 'emscripten' and sys_platform != 'win32') +pure-eval==0.2.3 +pygments==2.19.1 pytest==8.3.4 pytest-cov==6.0.0 +pytest-datadir==1.8.0 +pytest-regressions==2.8.2 +pyyaml==6.0.2 +stack-data==0.6.3 tomli==2.2.1 ; python_full_version <= '3.11' -typing-extensions==4.12.2 ; python_full_version < '3.11' +traitlets==5.14.3 +typing-extensions==4.12.2 ; python_full_version < '3.12' +wcwidth==0.2.13 diff --git a/requirements-only-tests-min-locked.txt b/requirements-only-tests-min-locked.txt index f0aaf57..9b660cf 100644 --- a/requirements-only-tests-min-locked.txt +++ b/requirements-only-tests-min-locked.txt @@ -6,5 +6,8 @@ iniconfig==2.0.0 packaging==24.2 pluggy==1.5.0 pytest==8.3.4 +pytest-datadir==1.8.0 +pytest-regressions==2.8.2 +pyyaml==6.0.2 tomli==2.2.1 ; python_full_version < '3.11' typing-extensions==4.12.2 ; python_full_version < '3.11' diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py index 5c0dce0..23ab07d 100644 --- a/src/example_fgen_basic/error_v/creation.py +++ b/src/example_fgen_basic/error_v/creation.py @@ -8,10 +8,10 @@ from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError try: - from example._lib import ( # type: ignore + from example_fgen_basic._lib import ( # type: ignore m_error_v_creation_w, ) -except (ModuleNotFoundError, ImportError) as exc: +except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover raise CompiledExtensionNotFoundError("example._lib.m_error_v_creation_w") from exc diff --git a/src/example_fgen_basic/error_v/error_v.py b/src/example_fgen_basic/error_v/error_v.py index 7b637d4..aa0652f 100644 --- a/src/example_fgen_basic/error_v/error_v.py +++ b/src/example_fgen_basic/error_v/error_v.py @@ -4,6 +4,8 @@ from __future__ import annotations +from typing import Any + from attrs import define from example_fgen_basic.pyfgen_runtime.base_finalisable import ( @@ -13,10 +15,10 @@ from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError try: - from example._lib import ( # type: ignore + from example_fgen_basic._lib import ( # type: ignore m_error_v_creation_w, ) -except (ModuleNotFoundError, ImportError) as exc: +except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover raise CompiledExtensionNotFoundError("example._lib.m_error_v_creation_w") from exc @@ -26,6 +28,15 @@ class ErrorV(FinalisableWrapperBase): TODO: auto docstring e.g. "Wrapper around the Fortran :class:`ErrorV`" """ + # Bug in Ipython pretty hence have to put this on every object? + def _repr_pretty_(self, p: Any, cycle: bool) -> None: + """ + Get pretty representation of self + + Used by IPython notebooks and other tools + """ + super()._repr_pretty_(p=p, cycle=cycle) + @property def exposed_attributes(self) -> tuple[str, ...]: """ diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py index 72ae5a6..7730a08 100644 --- a/tests/unit/test_error_v_creation.py +++ b/tests/unit/test_error_v_creation.py @@ -6,6 +6,9 @@ (deliberately, just to see how it goes). """ +import pytest +from IPython.lib.pretty import pretty + from example_fgen_basic.error_v import ErrorV from example_fgen_basic.error_v.creation import create_error @@ -38,10 +41,40 @@ def test_create_error_negative(): assert res.message == "Negative number supplied" -# Tests to write: -# - if we create more errors than we have available, we don't segfault. -# Instead, we should get an error back. -# That error should just use the instance ID of the last available array index -# (it is ok to overwrite an already used error to avoid a complete failure, -# but we should probably include that we did this in the error message). -# - we can resize the available number of error instances to avoid hitting limits +def test_error_too_many_instances(): + pytest.skip("Causes segfault right now") + # - if we create more errors than we have available, we don't segfault. + # Instead, we should get an error back. + # That error should just use the instance ID of the last available array index + # (it is ok to overwrite an already used error to avoid a complete failure, + # but we should probably include that we did this in the error message). + # TODO: expect error here + for _ in range(4097): + create_error(1) + + +def test_increase_number_of_instances(): + raise NotImplementedError + # - Make 4096 instances + # - show that making one more raises an error + # - increase number of instances + # - show that making one more now works without error + + +# Some test to illustrate what the formatting does +def test_error_str(file_regression): + res = create_error(1.0) + + file_regression.check(str(res)) + + +def test_error_pprint(file_regression): + res = create_error(1.0) + + file_regression.check(pretty(res)) + + +def test_error_html(file_regression): + res = create_error(1.0) + + file_regression.check(res._repr_html_(), extension=".html") diff --git a/tests/unit/test_error_v_creation/test_error_html.html b/tests/unit/test_error_v_creation/test_error_html.html new file mode 100644 index 0000000..d499603 --- /dev/null +++ b/tests/unit/test_error_v_creation/test_error_html.html @@ -0,0 +1,41 @@ +
+ +
+
+
ErrorV
+
instance_index=6
+ + + +
code0
message
+
+
+
diff --git a/tests/unit/test_error_v_creation/test_error_pprint.txt b/tests/unit/test_error_v_creation/test_error_pprint.txt new file mode 100644 index 0000000..502a0db --- /dev/null +++ b/tests/unit/test_error_v_creation/test_error_pprint.txt @@ -0,0 +1 @@ +ErrorV(instance_index=5, code=0, message=) diff --git a/tests/unit/test_error_v_creation/test_error_str.txt b/tests/unit/test_error_v_creation/test_error_str.txt new file mode 100644 index 0000000..9cc1073 --- /dev/null +++ b/tests/unit/test_error_v_creation/test_error_str.txt @@ -0,0 +1 @@ +ErrorV(instance_index=4, code=0, message=) From 71a2ae8211413b4e6158d18118bddf395024fd42 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Tue, 26 Aug 2025 11:00:35 +0200 Subject: [PATCH 08/49] Fix up location of manager and other bits --- meson.build | 1 + .../error_v/creation_wrapper.f90 | 40 +----------- src/example_fgen_basic/error_v/error_v.py | 8 +-- .../error_v/error_v_wrapper.f90 | 61 +++++++++++++++++++ 4 files changed, 67 insertions(+), 43 deletions(-) create mode 100644 src/example_fgen_basic/error_v/error_v_wrapper.f90 diff --git a/meson.build b/meson.build index 7a32154..0b605a9 100644 --- a/meson.build +++ b/meson.build @@ -52,6 +52,7 @@ if pyprojectwheelbuild_enabled # Injected with `script/inject-srcs-into-meson-build.py` srcs = files( 'src/example_fgen_basic/error_v/creation_wrapper.f90', + 'src/example_fgen_basic/error_v/error_v_wrapper.f90', 'src/example_fgen_basic/get_wavelength_wrapper.f90', ) diff --git a/src/example_fgen_basic/error_v/creation_wrapper.f90 b/src/example_fgen_basic/error_v/creation_wrapper.f90 index 417a91c..fa3c773 100644 --- a/src/example_fgen_basic/error_v/creation_wrapper.f90 +++ b/src/example_fgen_basic/error_v/creation_wrapper.f90 @@ -17,7 +17,7 @@ module m_error_v_creation_w implicit none private - public :: create_error, iget_code, iget_message + public :: create_error contains @@ -51,42 +51,4 @@ subroutine create_error(inv, res_instance_index) end subroutine create_error - ! Full set of wrapping strategies to pass different types in e.g. - ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 - ! (we will do a full re-write of the code which generates this, - ! but the strategies will probably stay as they are) - subroutine iget_code( & - instance_index, & - code & - ) - - integer, intent(in) :: instance_index - - integer, intent(out) :: code - - type(ErrorV), pointer :: instance - - call error_v_manager_associate_pointer_with_instance(instance_index, instance) - - code = instance % code - - end subroutine iget_code - - subroutine iget_message( & - instance_index, & - message & - ) - - integer, intent(in) :: instance_index - - character(len=128), intent(out) :: message - - type(ErrorV), pointer :: instance - - call error_v_manager_associate_pointer_with_instance(instance_index, instance) - - message = instance % message - - end subroutine iget_message - end module m_error_v_creation_w diff --git a/src/example_fgen_basic/error_v/error_v.py b/src/example_fgen_basic/error_v/error_v.py index aa0652f..11df613 100644 --- a/src/example_fgen_basic/error_v/error_v.py +++ b/src/example_fgen_basic/error_v/error_v.py @@ -16,10 +16,10 @@ try: from example_fgen_basic._lib import ( # type: ignore - m_error_v_creation_w, + m_error_v_w, ) except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover - raise CompiledExtensionNotFoundError("example._lib.m_error_v_creation_w") from exc + raise CompiledExtensionNotFoundError("example._lib.m_error_v_w") from exc @define @@ -57,7 +57,7 @@ def code(self) -> int: : Error code, retrieved from Fortran """ - code: int = m_error_v_creation_w.iget_code(instance_index=self.instance_index) + code: int = m_error_v_w.iget_code(instance_index=self.instance_index) return code @@ -72,7 +72,7 @@ def message(self) -> str: : Error message, retrieved from Fortran """ - message: str = m_error_v_creation_w.iget_message( + message: str = m_error_v_w.iget_message( instance_index=self.instance_index ).decode() diff --git a/src/example_fgen_basic/error_v/error_v_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_wrapper.f90 new file mode 100644 index 0000000..99ef037 --- /dev/null +++ b/src/example_fgen_basic/error_v/error_v_wrapper.f90 @@ -0,0 +1,61 @@ +!> Wrapper for interfacing `m_error_v` with Python +!> +!> Written by hand here. +!> Generation to be automated in future (including docstrings of some sort). +module m_error_v_w + + ! => allows us to rename on import to avoid clashes + use m_error_v, only: ErrorV + + ! The manager module, which makes this all work + use m_error_v_manager, only: & + get_free_instance_number, & + error_v_manager_associate_pointer_with_instance => associate_pointer_with_instance + ! TODO: build and finalisation + + implicit none + private + + public :: get_free_instance_number, iget_code, iget_message + +contains + + ! Full set of wrapping strategies to pass different types in e.g. + ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 + ! (we will do a full re-write of the code which generates this, + ! but the strategies will probably stay as they are) + subroutine iget_code( & + instance_index, & + code & + ) + + integer, intent(in) :: instance_index + + integer, intent(out) :: code + + type(ErrorV), pointer :: instance + + call error_v_manager_associate_pointer_with_instance(instance_index, instance) + + code = instance % code + + end subroutine iget_code + + subroutine iget_message( & + instance_index, & + message & + ) + + integer, intent(in) :: instance_index + + character(len=128), intent(out) :: message + + type(ErrorV), pointer :: instance + + call error_v_manager_associate_pointer_with_instance(instance_index, instance) + + message = instance % message + + end subroutine iget_message + +end module m_error_v_w From 129bcb9d1d7c52b3161a6abaa4460d2f47d1e581 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Tue, 26 Aug 2025 11:44:04 +0200 Subject: [PATCH 09/49] Add pointer-based variant --- meson.build | 2 +- src/example_fgen_basic/error_v/__init__.py | 4 +- src/example_fgen_basic/error_v/creation.py | 15 +- .../error_v/creation_wrapper.f90 | 31 +- src/example_fgen_basic/error_v/error_v.py | 62 +++ .../error_v/error_v_ptr_based_wrapper.f90 | 58 +++ src/example_fgen_basic/meson.build | 3 + .../pyfgen_runtime/base_finalisable.py | 131 ++++++ src/example_fgen_basic/runtime_helpers.py | 385 ------------------ tests/unit/main.f90 | 7 +- tests/unit/meson.build | 1 + tests/unit/test_error_v_creation.f90 | 72 ++++ tests/unit/test_error_v_creation.py | 32 +- .../test_error_ptr_based_pprint.txt | 1 + .../test_error_ptr_based_str.txt | 1 + tests/unit/test_get_wavelength.f90 | 2 +- 16 files changed, 413 insertions(+), 394 deletions(-) create mode 100644 src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 delete mode 100644 src/example_fgen_basic/runtime_helpers.py create mode 100644 tests/unit/test_error_v_creation.f90 create mode 100644 tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt create mode 100644 tests/unit/test_error_v_creation/test_error_ptr_based_str.txt diff --git a/meson.build b/meson.build index 0b605a9..54a178e 100644 --- a/meson.build +++ b/meson.build @@ -52,6 +52,7 @@ if pyprojectwheelbuild_enabled # Injected with `script/inject-srcs-into-meson-build.py` srcs = files( 'src/example_fgen_basic/error_v/creation_wrapper.f90', + 'src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90', 'src/example_fgen_basic/error_v/error_v_wrapper.f90', 'src/example_fgen_basic/get_wavelength_wrapper.f90', ) @@ -81,7 +82,6 @@ if pyprojectwheelbuild_enabled 'src/example_fgen_basic/pyfgen_runtime/base_finalisable.py', 'src/example_fgen_basic/pyfgen_runtime/exceptions.py', 'src/example_fgen_basic/pyfgen_runtime/formatting.py', - 'src/example_fgen_basic/runtime_helpers.py', ) # The ancillary library, diff --git a/src/example_fgen_basic/error_v/__init__.py b/src/example_fgen_basic/error_v/__init__.py index 4182384..ddd2b4a 100644 --- a/src/example_fgen_basic/error_v/__init__.py +++ b/src/example_fgen_basic/error_v/__init__.py @@ -2,6 +2,6 @@ Definition of an error value """ -from example_fgen_basic.error_v.error_v import ErrorV +from example_fgen_basic.error_v.error_v import ErrorV, ErrorVPtrBased -__all__ = ["ErrorV"] +__all__ = ["ErrorV", "ErrorVPtrBased"] diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py index 23ab07d..e62c2c6 100644 --- a/src/example_fgen_basic/error_v/creation.py +++ b/src/example_fgen_basic/error_v/creation.py @@ -4,7 +4,7 @@ from __future__ import annotations -from example_fgen_basic.error_v.error_v import ErrorV +from example_fgen_basic.error_v.error_v import ErrorV, ErrorVPtrBased from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError try: @@ -24,3 +24,16 @@ def create_error(inv: int) -> ErrorV: error = ErrorV(instance_index) return error + + +def create_error_ptr_based(inv: int) -> ErrorVPtrBased: + """ + Create an instance of error (a wrapper around our Fortran derived type) + + Uses the pointer based logic + """ + instance_ptr = m_error_v_creation_w.create_error_ptr_based(inv) + + error = ErrorVPtrBased(instance_ptr) + + return error diff --git a/src/example_fgen_basic/error_v/creation_wrapper.f90 b/src/example_fgen_basic/error_v/creation_wrapper.f90 index fa3c773..51d3287 100644 --- a/src/example_fgen_basic/error_v/creation_wrapper.f90 +++ b/src/example_fgen_basic/error_v/creation_wrapper.f90 @@ -4,6 +4,8 @@ !> Generation to be automated in future (including docstrings of some sort). module m_error_v_creation_w + use iso_c_binding, only: c_ptr, c_loc + ! => allows us to rename on import to avoid clashes use m_error_v_creation, only: o_create_error => create_error use m_error_v, only: ErrorV @@ -17,7 +19,7 @@ module m_error_v_creation_w implicit none private - public :: create_error + public :: create_error, create_error_ptr_based contains @@ -51,4 +53,31 @@ subroutine create_error(inv, res_instance_index) end subroutine create_error + subroutine create_error_ptr_based(inv, res_instance_ptr) + ! Needs to be subroutine to have the created instance persist I think + ! (we can check) + ! function create_error(inv) result(res_instance_index) + + integer, intent(in) :: inv + !! Input value to use to create the error + + !f2py integer(8), intent(out) :: res_instance_ptr + type(c_ptr), intent(out) :: res_instance_ptr + !! Pointer to the resulting instance + ! + ! This is the major trick for wrapping. + ! We return pointers (passed as integers) to Python rather than the instance itself. + + type(ErrorV), pointer :: res + + ! Question is: when does this get deallocated? + ! When we go out of scope? + ! If yes, that will be why we had to do this array thing. + allocate(res) + res = o_create_error(inv) + + res_instance_ptr = c_loc(res) + + end subroutine create_error_ptr_based + end module m_error_v_creation_w diff --git a/src/example_fgen_basic/error_v/error_v.py b/src/example_fgen_basic/error_v/error_v.py index 11df613..c09d9ea 100644 --- a/src/example_fgen_basic/error_v/error_v.py +++ b/src/example_fgen_basic/error_v/error_v.py @@ -10,12 +10,15 @@ from example_fgen_basic.pyfgen_runtime.base_finalisable import ( FinalisableWrapperBase, + FinalisableWrapperBasePtrBased, check_initialised, + check_initialised_ptr_based, ) from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError try: from example_fgen_basic._lib import ( # type: ignore + m_error_v_ptr_based_w, m_error_v_w, ) except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover @@ -77,3 +80,62 @@ def message(self) -> str: ).decode() return message + + +@define +class ErrorVPtrBased(FinalisableWrapperBasePtrBased): + """ + TODO: auto docstring e.g. "Wrapper around the Fortran :class:`ErrorV`" + + Uses the pointer-based passing logic + """ + + # Bug in Ipython pretty hence have to put this on every object? + def _repr_pretty_(self, p: Any, cycle: bool) -> None: + """ + Get pretty representation of self + + Used by IPython notebooks and other tools + """ + super()._repr_pretty_(p=p, cycle=cycle) + + @property + def exposed_attributes(self) -> tuple[str, ...]: + """ + Attributes exposed by this wrapper + """ + return ("code", "message") + + # TODO: from_build_args, from_new_connection, context manager, finalise + + @property + @check_initialised_ptr_based + def code(self) -> int: + """ + Error code + + Returns + ------- + : + Error code, retrieved from Fortran + """ + code: int = m_error_v_ptr_based_w.iget_code(instance_ptr=self.instance_ptr) + + return code + + @property + @check_initialised_ptr_based + def message(self) -> str: + """ + Error message + + Returns + ------- + : + Error message, retrieved from Fortran + """ + message: str = m_error_v_ptr_based_w.iget_message( + instance_ptr=self.instance_ptr + ).decode() + + return message diff --git a/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 new file mode 100644 index 0000000..ca9c07e --- /dev/null +++ b/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 @@ -0,0 +1,58 @@ +!> Wrapper for interfacing `m_error_v` with Python, using pointer only (no instance array) +!> +!> Written by hand here. +!> Generation to be automated in future (including docstrings of some sort). +module m_error_v_ptr_based_w + + use iso_c_binding, only: c_ptr, c_f_pointer + + use m_error_v, only: ErrorV + + implicit none + private + + public :: iget_code, iget_message + +contains + + ! Full set of wrapping strategies to pass different types in e.g. + ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 + ! (we will do a full re-write of the code which generates this, + ! but the strategies will probably stay as they are) + subroutine iget_code( & + instance_ptr, & + code & + ) + + !f2py integer(8), intent(in) :: instance_ptr + type(c_ptr), intent(in) :: instance_ptr + + integer, intent(out) :: code + + type(ErrorV), pointer :: instance + + call c_f_pointer(instance_ptr, instance) + + code = instance % code + + end subroutine iget_code + + subroutine iget_message( & + instance_ptr, & + message & + ) + + !f2py integer(8), intent(in) :: instance_ptr + type(c_ptr), intent(in) :: instance_ptr + + character(len=128), intent(out) :: message + + type(ErrorV), pointer :: instance + + call c_f_pointer(instance_ptr, instance) + + message = instance % message + + end subroutine iget_message + +end module m_error_v_ptr_based_w diff --git a/src/example_fgen_basic/meson.build b/src/example_fgen_basic/meson.build index 741b8be..8c67049 100644 --- a/src/example_fgen_basic/meson.build +++ b/src/example_fgen_basic/meson.build @@ -1,4 +1,7 @@ srcs += files( + 'error_v/creation.f90', + 'error_v/error_v.f90', + 'fpyfgen/base_finalisable.f90', 'get_wavelength.f90', 'kind_parameters.f90', ) diff --git a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py index dd3e80a..afee3d0 100644 --- a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py +++ b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py @@ -163,3 +163,134 @@ def checked( return method(ref, *args, **kwargs) return checked # type: ignore + + +@define +class FinalisableWrapperBasePtrBased(ABC): + """ + Base class for Fortran derived type wrappers using the pointer-based passing + """ + + instance_ptr: int | None = None + """ + Pointer to Fortran instance (technically a C pointer) + """ + + def __str__(self) -> str: + """ + Get string representation of self + """ + return to_str( + self, + self.exposed_attributes, + ) + + def _repr_pretty_(self, p: Any, cycle: bool) -> None: + """ + Get pretty representation of self + + Used by IPython notebooks and other tools + """ + to_pretty( + self, + self.exposed_attributes, + p=p, + cycle=cycle, + ) + + def _repr_html_(self) -> str: + """ + Get html representation of self + + Used by IPython notebooks and other tools + """ + return to_html( + self, + self.exposed_attributes, + ) + + @property + def initialized(self) -> bool: + """ + Is the instance initialised, i.e. connected to a Fortran instance? + """ + return self.instance_ptr is not None + + @property + @abstractmethod + def exposed_attributes(self) -> tuple[str, ...]: + """ + Attributes exposed by this wrapper + """ + ... + + # TODO: consider whether we need these + # @classmethod + # @abstractmethod + # def from_new_connection(cls) -> FinalisableWrapperBase: + # """ + # Initialise by establishing a new connection with the Fortran module + # + # This requests a new model index from the Fortran module and then + # initialises a class instance + # + # Returns + # ------- + # New class instance + # """ + # ... + # + # @abstractmethod + # def finalize(self) -> None: + # """ + # Finalise the Fortran instance and set self back to being uninitialised + # + # This method resets `self.instance_ptr` back to + # `None` + # + # Should be decorated with :func:`check_initialised` + # """ + # # call to Fortran module goes here when implementing + # self._uninitialise_instance_index() + + def _uninitialise_instance_index(self) -> None: + self.instance_index = None + + +WrapperPtrBased = TypeVar("WrapperPtrBased", bound=FinalisableWrapperBasePtrBased) + + +def check_initialised_ptr_based( + method: Callable[Concatenate[WrapperPtrBased, P], T], +) -> Callable[Concatenate[WrapperPtrBased, P], T]: + """ + Check that the wrapper object has been initialised before executing the method + + Parameters + ---------- + method + Method to wrap + + Returns + ------- + : + Wrapped method + + Raises + ------ + InitialisationError + Wrapper is not initialised + """ + + @wraps(method) + def checked( + ref: WrapperPtrBased, + *args: P.args, + **kwargs: P.kwargs, + ) -> Any: + if not ref.initialized: + raise NotInitialisedError(ref, method) + + return method(ref, *args, **kwargs) + + return checked # type: ignore diff --git a/src/example_fgen_basic/runtime_helpers.py b/src/example_fgen_basic/runtime_helpers.py deleted file mode 100644 index 3249812..0000000 --- a/src/example_fgen_basic/runtime_helpers.py +++ /dev/null @@ -1,385 +0,0 @@ -""" -Runtime helpers - -These would be moved to fgen-runtime or a similar package -""" - -from __future__ import annotations - -from abc import ABC, abstractmethod -from collections.abc import Iterable -from functools import wraps -from typing import Any, Callable, TypeVar - -import attrs -from attrs import define, field -from typing_extensions import Concatenate, ParamSpec - -from example_fgen_basic.exceptions import NotInitialisedError, UnallocatedMemoryError - -# Might be needed for Python 3.9 -# from typing_extensions import Concatenate, ParamSpec - - -# TODO: move this section to formatting module - - -def get_attribute_str_value(instance: FinalisableWrapperBase, attribute: str) -> str: - """ - Get the string version of an attribute's value - - Parameters - ---------- - instance - Instance from which to get the attribute - - attribute - Attribute for which to get the value - - Returns - ------- - String version of the attribute's value, with graceful handling of errors. - """ - try: - return f"{attribute}={getattr(instance, attribute)}" - except UnallocatedMemoryError: - # TODO: change this when we move to better error handling - return f"{attribute} is unallocated" - - -def to_str(instance: FinalisableWrapperBase, exposed_attributes: Iterable[str]) -> str: - """ - Convert an instance to its string representation - - Parameters - ---------- - instance - Instance to convert - - exposed_attributes - Attributes from Fortran that the instance exposes - - Returns - ------- - String representation of the instance - """ - if not instance.initialized: - return f"Uninitialised {instance!r}" - - if not exposed_attributes: - return repr(instance) - - attribute_values = [ - get_attribute_str_value(instance, v) for v in exposed_attributes - ] - - return f"{repr(instance)[:-1]}, {', '.join(attribute_values)})" - - -def to_pretty( - instance: FinalisableWrapperBase, - exposed_attributes: Iterable[str], - p: Any, - cycle: bool, - indent: int = 4, -) -> None: - """ - Pretty-print an instance - - Parameters - ---------- - instance - Instance to convert - - exposed_attributes - Attributes from Fortran that the instance exposes - - p - Pretty printing object - - cycle - Whether the pretty printer has detected a cycle or not. - - indent - Indent to apply to the pretty printing group - """ - if not instance.initialized: - p.text(str(instance)) - return - - if not exposed_attributes: - p.text(str(instance)) - return - - with p.group(indent, f"{repr(instance)[:-1]}", ")"): - for att in exposed_attributes: - p.text(",") - p.breakable() - - p.text(get_attribute_str_value(instance, att)) - - -def add_attribute_row( - attribute_name: str, attribute_value: str, attribute_rows: list[str] -) -> list[str]: - """ - Add a row for displaying an attribute's value to a list of rows - - Parameters - ---------- - attribute_name - Attribute's name - - attribute_value - Attribute's value - - attribute_rows - Existing attribute rows - - - Returns - ------- - Attribute rows, with the new row appended - """ - attribute_rows.append( - f"{attribute_name}{attribute_value}" # noqa: E501 - ) - - return attribute_rows - - -def to_html(instance: FinalisableWrapperBase, exposed_attributes: Iterable[str]) -> str: - """ - Convert an instance to its html representation - - Parameters - ---------- - instance - Instance to convert - - exposed_attributes - Attributes from Fortran that the instance exposes - - Returns - ------- - HTML representation of the instance - """ - if not instance.initialized: - return str(instance) - - if not exposed_attributes: - return str(instance) - - instance_class_name = repr(instance).split("(")[0] - - attribute_rows: list[str] = [] - for att in exposed_attributes: - try: - att_val = getattr(instance, att) - except UnallocatedMemoryError: - # TODO: change this when we move to better error handling - att_val = "Unallocated" - attribute_rows = add_attribute_row(att, att_val, attribute_rows) - continue - - try: - att_val = att_val._repr_html_() - except AttributeError: - att_val = str(att_val) - - attribute_rows = add_attribute_row(att, att_val, attribute_rows) - - attribute_rows_for_table = "\n ".join(attribute_rows) - - css_style = """.fgen-wrap { - /*font-family: monospace;*/ - width: 540px; -} - -.fgen-header { - padding: 6px 0 6px 3px; - border-bottom: solid 1px #777; - color: #555;; -} - -.fgen-header > div { - display: inline; - margin-top: 0; - margin-bottom: 0; -} - -.fgen-basefinalizable-cls, -.fgen-basefinalizable-instance-index { - margin-left: 2px; - margin-right: 10px; -} - -.fgen-basefinalizable-cls { - font-weight: bold; - color: #000000; -}""" - - return "\n".join( - [ - "
", - " ", - "
", - "
", - f"
{instance_class_name}
", - f"
instance_index={instance.instance_index}
", # noqa: E501 - " ", - f" {attribute_rows_for_table}", - "
", - "
", - "
", - "
", - ] - ) - - -# End of stuff to move to formatting module - -INVALID_INSTANCE_INDEX: int = -1 -""" -Value used to denote an invalid ``instance_index``. - -This can occur value when a wrapper class -has not yet been initialised (connected to a Fortran instance). -""" - - -@define -class FinalisableWrapperBase(ABC): - """ - Base class for Fortran derived type wrappers - """ - - instance_index: int = field( - validator=attrs.validators.instance_of(int), - default=INVALID_INSTANCE_INDEX, - ) - """ - Model index of wrapper Fortran instance - """ - - def __str__(self) -> str: - """ - Get string representation of self - """ - return to_str( - self, - self.exposed_attributes, - ) - - def _repr_pretty_(self, p: Any, cycle: bool) -> None: - """ - Get pretty representation of self - - Used by IPython notebooks and other tools - """ - to_pretty( - self, - self.exposed_attributes, - p=p, - cycle=cycle, - ) - - def _repr_html_(self) -> str: - """ - Get html representation of self - - Used by IPython notebooks and other tools - """ - return to_html( - self, - self.exposed_attributes, - ) - - @property - def initialized(self) -> bool: - """ - Is the instance initialised, i.e. connected to a Fortran instance? - """ - return self.instance_index != INVALID_INSTANCE_INDEX - - @property - @abstractmethod - def exposed_attributes(self) -> tuple[str, ...]: - """ - Attributes exposed by this wrapper - """ - ... - - # @classmethod - # @abstractmethod - # def from_new_connection(cls) -> FinalisableWrapperBase: - # """ - # Initialise by establishing a new connection with the Fortran module - # - # This requests a new model index from the Fortran module and then - # initialises a class instance - # - # Returns - # ------- - # New class instance - # """ - # ... - # - # @abstractmethod - # def finalize(self) -> None: - # """ - # Finalise the Fortran instance and set self back to being uninitialised - # - # This method resets ``self.instance_index`` back to - # ``_UNINITIALISED_instance_index`` - # - # Should be decorated with :func:`check_initialised` - # """ - # # call to Fortran module goes here when implementing - # self._uninitialise_instance_index() - - def _uninitialise_instance_index(self) -> None: - self.instance_index = INVALID_INSTANCE_INDEX - - -P = ParamSpec("P") -T = TypeVar("T") -Wrapper = TypeVar("Wrapper", bound=FinalisableWrapperBase) - - -def check_initialised( - method: Callable[Concatenate[Wrapper, P], T], -) -> Callable[Concatenate[Wrapper, P], T]: - """ - Check that the wrapper object has been initialised before executing the method - - Parameters - ---------- - method - Method to wrap - - Returns - ------- - : - Wrapped method - - Raises - ------ - InitialisationError - Wrapper is not initialised - """ - - @wraps(method) - def checked( - ref: Wrapper, - *args: P.args, - **kwargs: P.kwargs, - ) -> Any: - if not ref.initialized: - raise NotInitialisedError(ref, method) - - return method(ref, *args, **kwargs) - - return checked # type: ignore diff --git a/tests/unit/main.f90 b/tests/unit/main.f90 index 5f4d22f..879fe24 100644 --- a/tests/unit/main.f90 +++ b/tests/unit/main.f90 @@ -3,6 +3,8 @@ program tester_unit use, intrinsic :: iso_fortran_env, only: error_unit use testdrive, only: run_testsuite, new_testsuite, testsuite_type, select_suite, run_selected, get_argument + + use test_error_v_creation, only: collect_error_v_creation_tests use test_get_wavelength, only: collect_get_wavelength_tests implicit none @@ -14,7 +16,10 @@ program tester_unit stat = 0 ! add new tests here - testsuites = [new_testsuite("test_get_wavelength", collect_get_wavelength_tests)] + testsuites = [ & + new_testsuite("test_get_wavelength", collect_get_wavelength_tests), & + new_testsuite("test_error_v_creation", collect_error_v_creation_tests) & + ] call get_argument(1, suite_name) call get_argument(2, test_name) diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 4e69e7c..fb79a84 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -7,6 +7,7 @@ testdrive_dep = dependency( test_file_stubs = [ 'test_get_wavelength', + 'test_error_v_creation', ] test_srcs = files( diff --git a/tests/unit/test_error_v_creation.f90 b/tests/unit/test_error_v_creation.f90 new file mode 100644 index 0000000..61e3ce5 --- /dev/null +++ b/tests/unit/test_error_v_creation.f90 @@ -0,0 +1,72 @@ +!> Tests of m_error_v_creation +module test_error_v_creation + + ! How to print to stdout + use ISO_Fortran_env, only: stdout => OUTPUT_UNIT + use testdrive, only: new_unittest, unittest_type, error_type, check + + use kind_parameters, only: dp + + implicit none + private + + public :: collect_error_v_creation_tests + +contains + + subroutine collect_error_v_creation_tests(testsuite) + !> Collection of tests + type(unittest_type), allocatable, intent(out) :: testsuite(:) + + testsuite = [ & + new_unittest("test_error_v_creation_basic", test_error_v_creation_basic), & + new_unittest("test_error_v_creation_edge", test_error_v_creation_edge) & + ] + + end subroutine collect_error_v_creation_tests + + subroutine test_error_v_creation_basic(error) + use m_error_v, only: ErrorV + use m_error_v_creation, only: create_error + + type(error_type), allocatable, intent(out) :: error + + type(ErrorV) :: res + + res = create_error(1) + + ! ! How to print to stdout + ! write( stdout, '(e13.4e2)') res + ! write( stdout, '(e13.4e2)') exp + + call check(error, res % code, 0) + call check(error, res % message, "") + + end subroutine test_error_v_creation_basic + + subroutine test_error_v_creation_edge(error) + use m_error_v, only: ErrorV + use m_error_v_creation, only: create_error + + type(error_type), allocatable, intent(out) :: error + + ! type(ErrorV), target :: res + ! type(ErrorV), pointer :: res_ptr + ! + ! res = create_error(1) + ! res_ptr => res + type(ErrorV), pointer :: res + + allocate(res) + res = create_error(1) + + ! ! How to print to stdout + ! write( stdout, '(e13.4e2)') res + ! write( stdout, '(e13.4e2)') exp + + call check(error, res % code, 0) + call check(error, res % message, "") + + end subroutine test_error_v_creation_edge + +end module test_error_v_creation diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py index 7730a08..a23df1b 100644 --- a/tests/unit/test_error_v_creation.py +++ b/tests/unit/test_error_v_creation.py @@ -9,8 +9,8 @@ import pytest from IPython.lib.pretty import pretty -from example_fgen_basic.error_v import ErrorV -from example_fgen_basic.error_v.creation import create_error +from example_fgen_basic.error_v import ErrorV, ErrorVPtrBased +from example_fgen_basic.error_v.creation import create_error, create_error_ptr_based def test_create_error_odd(): @@ -78,3 +78,31 @@ def test_error_html(file_regression): res = create_error(1.0) file_regression.check(res._repr_html_(), extension=".html") + + +def test_create_error_even_ptr_based(): + res = create_error_ptr_based(2.0) + + assert isinstance(res, ErrorVPtrBased) + + assert res.code != 0 + assert res.code == 1 + assert res.message == "Even number supplied" + + +def test_error_ptr_based_str(file_regression): + res = create_error_ptr_based(1.0) + + file_regression.check(str(res)) + + +def test_error_ptr_based_pprint(file_regression): + res = create_error_ptr_based(1.0) + + file_regression.check(pretty(res)) + + +def test_error_ptr_based_html(file_regression): + res = create_error_ptr_based(1.0) + + file_regression.check(res._repr_html_(), extension=".html") diff --git a/tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt b/tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt new file mode 100644 index 0000000..6f373c0 --- /dev/null +++ b/tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt @@ -0,0 +1 @@ +ErrorVPtrBased(instance_ptr=105553174233376, code=0, message=) diff --git a/tests/unit/test_error_v_creation/test_error_ptr_based_str.txt b/tests/unit/test_error_v_creation/test_error_ptr_based_str.txt new file mode 100644 index 0000000..21467d4 --- /dev/null +++ b/tests/unit/test_error_v_creation/test_error_ptr_based_str.txt @@ -0,0 +1 @@ +ErrorVPtrBased(instance_ptr=105553174233232, code=0, message=) diff --git a/tests/unit/test_get_wavelength.f90 b/tests/unit/test_get_wavelength.f90 index 99cba3f..1b91fad 100644 --- a/tests/unit/test_get_wavelength.f90 +++ b/tests/unit/test_get_wavelength.f90 @@ -1,4 +1,4 @@ -!> Tests of get_wavelength +!> Tests of m_get_wavelength module test_get_wavelength ! How to print to stdout From c8a72d101124fdf65ef7d8d56f5e4e953892a89c Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Tue, 26 Aug 2025 16:05:47 +0200 Subject: [PATCH 10/49] Finish mucking around with pointer based option --- src/example_fgen_basic/error_v/error_v.f90 | 2 + src/example_fgen_basic/error_v/error_v.py | 71 +++++++++++ .../error_v/error_v_ptr_based_wrapper.f90 | 114 ++++++++++++++++- src/example_fgen_basic/exceptions.py | 2 +- .../pyfgen_runtime/base_finalisable.py | 119 ++++++++++++------ .../pyfgen_runtime/exceptions.py | 2 +- .../pyfgen_runtime/formatting.py | 6 +- tests/unit/test_error_v.py | 31 +++++ 8 files changed, 305 insertions(+), 42 deletions(-) create mode 100644 tests/unit/test_error_v.py diff --git a/src/example_fgen_basic/error_v/error_v.f90 b/src/example_fgen_basic/error_v/error_v.f90 index 166be59..47bd788 100644 --- a/src/example_fgen_basic/error_v/error_v.f90 +++ b/src/example_fgen_basic/error_v/error_v.f90 @@ -92,6 +92,8 @@ subroutine finalise(self) ! Hopefully can leave without docstring (like Python) ! If we make message allocatable, deallocate here + self % code = 1 + self % message = "" end subroutine finalise diff --git a/src/example_fgen_basic/error_v/error_v.py b/src/example_fgen_basic/error_v/error_v.py index c09d9ea..9083c16 100644 --- a/src/example_fgen_basic/error_v/error_v.py +++ b/src/example_fgen_basic/error_v/error_v.py @@ -13,6 +13,7 @@ FinalisableWrapperBasePtrBased, check_initialised, check_initialised_ptr_based, + execute_finalise_on_fail_ptr_based, ) from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError @@ -106,7 +107,77 @@ def exposed_attributes(self) -> tuple[str, ...]: """ return ("code", "message") + @classmethod + def from_new_connection(cls) -> ErrorVPtrBased: + """ + Initialise from a new connection + + The user is responsible for releasing this connection + using :attr:`~finalise` when it is no longer needed. + Alternatively a :obj:`~AtmosphereToOceanCarbonFluxCalculatorContext` + can be used to handle the finalisation using a context manager. + + Returns + ------- + A new instance with a unique instance index + + Raises + ------ + WrapperErrorUnknownCause + If a new instance could not be allocated + + This could occur if too many instances are allocated at any one time + """ + instance_ptr = m_error_v_ptr_based_w.get_instance_ptr() + # TODO: result type handling here + + return cls(instance_ptr) + # TODO: from_build_args, from_new_connection, context manager, finalise + @classmethod + def from_build_args( + cls, + code: int, + message: str = "", + ) -> ErrorVPtrBased: + """ + Build the class (including connecting to Fortran) + """ + out = cls.from_new_connection() + # TODO: remove or update this construct when we have result types + execute_finalise_on_fail_ptr_based( + out, + m_error_v_ptr_based_w.instance_build, + code=code, + message=message, + ) + + return out + + @check_initialised + def finalise(self) -> None: + """ + Close the connection with the Fortran module + """ + m_error_v_ptr_based_w.instance_finalise(self.instance_ptr) + self._uninitialise_instance_ptr() + + @property + def is_associated(self) -> bool: + """ + Check whether `self`'s pointer is associated on the Fortran side + + Returns + ------- + : + Whether `self`'s pointer is associated on the Fortran side + """ + if self.instance_ptr is None: + return False + + res: bool = bool(m_error_v_ptr_based_w.is_associated(self.instance_ptr)) # type: ignore + + return res @property @check_initialised_ptr_based diff --git a/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 index ca9c07e..2f59218 100644 --- a/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 +++ b/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 @@ -4,17 +4,127 @@ !> Generation to be automated in future (including docstrings of some sort). module m_error_v_ptr_based_w - use iso_c_binding, only: c_ptr, c_f_pointer + use iso_c_binding, only: c_f_pointer, c_loc, c_null_ptr, c_ptr use m_error_v, only: ErrorV implicit none private - public :: iget_code, iget_message + public :: get_instance_ptr, instance_build, instance_finalise, is_associated, & + iget_code, iget_message contains + subroutine get_instance_ptr(res_instance_ptr) + !> Get a pointer to a new instance + ! + ! Needs to be subroutine to have the created instance persist I think + ! (we can check) + ! function create_error(inv) result(res_instance_index) + + !f2py integer(8), intent(out) :: res_instance_ptr + type(c_ptr), intent(out) :: res_instance_ptr + !! Pointer to the resulting instance + ! + ! This is the major trick for wrapping. + ! We return pointers (passed as integers) to Python rather than the instance itself. + + type(ErrorV), pointer :: res + ! Question is: when does this get deallocated? + ! When we go out of scope? + ! If yes, that will be why we had to do this array thing. + allocate(res) + + res_instance_ptr = c_loc(res) + + end subroutine get_instance_ptr + + subroutine instance_build(instance_ptr, code, message) + !> Build an instance + + !f2py integer(8), intent(in) :: instance_ptr + type(c_ptr), intent(in) :: instance_ptr + !! Pointer to the instance + ! + ! This is the major trick for wrapping. + ! We pass pointers (passed as integers) to Python rather than the instance itself. + + integer, intent(in) :: code + character(len=*), optional, intent(in) :: message + + type(ErrorV), pointer :: inst + + call c_f_pointer(instance_ptr, inst) + + call inst % build(code, message) + + end subroutine instance_build + + subroutine instance_finalise(instance_ptr) + !> Finalise an instance + + !f2py integer(8), intent(inout) :: instance_ptr + type(c_ptr), intent(inout) :: instance_ptr + !! Pointer to the instance + ! + ! This is the major trick for wrapping. + ! We pass pointers (passed as integers) to Python rather than the instance itself. + + type(ErrorV), pointer :: inst + + call c_f_pointer(instance_ptr, inst) + + ! This may be why we used the array approach. + ! The issue here is that, if you call this method twice, + ! there is no way to work out that you're the 'second caller'. + ! When the first call calls `deallocate(inst)`, + ! this puts any other pointers to the instance in an undefined status + ! (https://www.ibm.com/docs/en/xl-fortran-aix/16.1.0?topic=attributes-deallocate). + ! The result of calling associated on an undefined pointer + ! can be anything (https://stackoverflow.com/questions/72140217/can-you-test-for-nullpointers-in-fortran), + ! i.e. there is no way to tell that someone else + ! has already called finalise before you have. + ! This also explains the undefined status issue nicely: + ! community.intel.com/t5/Intel-Fortran-Compiler/DEALLOCATING-DATA-TYPE-POINTERS/m-p/982338#M100027 + ! + ! We'd have to introduce some reference counter to make this work I think. + ! Probably better advice for now, don't share pointer values + ! on the Python side, you have to be super careful about uninitialising if you do. + ! Avoiding pointers and using allocatable instead + ! was probably the other reason we did it how we did + ! community.intel.com/t5/Intel-Fortran-Compiler/how-to-test-if-pointer-array-is-allocated/m-p/1138643#M136486. + if (associated(inst)) then + call inst % finalise() + deallocate(inst) + end if + + end subroutine instance_finalise + + subroutine is_associated(instance_ptr, res) + !> Check if a pointer is associated with an instance + + !f2py integer(8), intent(in) :: instance_ptr + type(c_ptr), intent(in) :: instance_ptr + !! Pointer to the instance + ! + ! This is the major trick for wrapping. + ! We pass pointers (passed as integers) to Python rather than the instance itself. + + logical, intent(out) :: res + !! Whether `instance_ptr` is associated or not + + type(ErrorV), pointer :: inst + + call c_f_pointer(instance_ptr, inst) + + print *, instance_ptr + print *, inst + res = associated(inst) + print *, res + + end subroutine is_associated + ! Full set of wrapping strategies to pass different types in e.g. ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 ! (we will do a full re-write of the code which generates this, diff --git a/src/example_fgen_basic/exceptions.py b/src/example_fgen_basic/exceptions.py index 85ca24e..5de3212 100644 --- a/src/example_fgen_basic/exceptions.py +++ b/src/example_fgen_basic/exceptions.py @@ -57,7 +57,7 @@ def __init__(self, instance: Any, method: Optional[Callable[..., Any]] = None): if method: error_msg = f"{instance} must be initialised before {method} is called" else: - error_msg = f"instance ({instance:r}) is not initialized yet" + error_msg = f"instance ({instance:r}) is not initialised yet" super().__init__(error_msg) diff --git a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py index afee3d0..9dc45c2 100644 --- a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py +++ b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py @@ -77,7 +77,7 @@ def _repr_html_(self) -> str: ) @property - def initialized(self) -> bool: + def initialised(self) -> bool: """ Is the instance initialised, i.e. connected to a Fortran instance? """ @@ -108,7 +108,7 @@ def exposed_attributes(self) -> tuple[str, ...]: # ... # # @abstractmethod - # def finalize(self) -> None: + # def finalise(self) -> None: # """ # Finalise the Fortran instance and set self back to being uninitialised # @@ -157,7 +157,7 @@ def checked( *args: P.args, **kwargs: P.kwargs, ) -> Any: - if not ref.initialized: + if not ref.initialised: raise NotInitialisedError(ref, method) return method(ref, *args, **kwargs) @@ -210,7 +210,7 @@ def _repr_html_(self) -> str: ) @property - def initialized(self) -> bool: + def initialised(self) -> bool: """ Is the instance initialised, i.e. connected to a Fortran instance? """ @@ -224,37 +224,36 @@ def exposed_attributes(self) -> tuple[str, ...]: """ ... - # TODO: consider whether we need these - # @classmethod - # @abstractmethod - # def from_new_connection(cls) -> FinalisableWrapperBase: - # """ - # Initialise by establishing a new connection with the Fortran module - # - # This requests a new model index from the Fortran module and then - # initialises a class instance - # - # Returns - # ------- - # New class instance - # """ - # ... - # - # @abstractmethod - # def finalize(self) -> None: - # """ - # Finalise the Fortran instance and set self back to being uninitialised - # - # This method resets `self.instance_ptr` back to - # `None` - # - # Should be decorated with :func:`check_initialised` - # """ - # # call to Fortran module goes here when implementing - # self._uninitialise_instance_index() + @classmethod + @abstractmethod + def from_new_connection(cls) -> FinalisableWrapperBase: + """ + Initialise by establishing a new connection with the Fortran module - def _uninitialise_instance_index(self) -> None: - self.instance_index = None + This requests a new model index from the Fortran module and then + initialises a class instance. + + Returns + ------- + : + New class instance + """ + ... + + @abstractmethod + def finalise(self) -> None: + """ + Finalise the Fortran instance and set self back to being uninitialised + + This method resets `self.instance_ptr` back to `None`. + + Should be decorated with :func:`check_initialised` + """ + # call to Fortran module goes here when implementing + self._uninitialise_instance_ptr() + + def _uninitialise_instance_ptr(self) -> None: + self.instance_ptr = None WrapperPtrBased = TypeVar("WrapperPtrBased", bound=FinalisableWrapperBasePtrBased) @@ -288,9 +287,59 @@ def checked( *args: P.args, **kwargs: P.kwargs, ) -> Any: - if not ref.initialized: + if not ref.initialised: raise NotInitialisedError(ref, method) return method(ref, *args, **kwargs) return checked # type: ignore + + +# Thank you for type hints info +# https://adamj.eu/tech/2021/05/11/python-type-hints-args-and-kwargs/ +def execute_finalise_on_fail_ptr_based( + inst: FinalisableWrapperBasePtrBased, + func_to_try: Callable[Concatenate[int, P], T], + *args: P.args, + **kwargs: P.kwargs, +) -> T: + """ + Execute a function, finalising the instance before raising if any error occurs + + This function is most useful in factory functions where it provides a + clean way of ensuring that any Fortran is freed up in the event of an + initialisation failure for any reason + + Parameters + ---------- + inst + Instance whose model index we will use when executing the functin + + func_to_try + Function to try executing, must take `inst`'s instance pointer as its + first argument + + *args + Passed to `func_to_try` + + **kwargs + Passed to `func_to_try` + + Returns + ------- + : + Result of calling `func_to_try(inst.instance_ptr, *args, **kwargs)` + + Raises + ------ + Exception + Any exception which occurs when calling `func_to_try`. Before the + exception is raised, `inst.finalise()` is called. + """ + try: + return func_to_try(inst.instance_ptr, *args, **kwargs) + except RuntimeError: + # finalise the instance before raising + inst.finalise() + + raise diff --git a/src/example_fgen_basic/pyfgen_runtime/exceptions.py b/src/example_fgen_basic/pyfgen_runtime/exceptions.py index 0dbf4f7..0edd2ed 100644 --- a/src/example_fgen_basic/pyfgen_runtime/exceptions.py +++ b/src/example_fgen_basic/pyfgen_runtime/exceptions.py @@ -56,7 +56,7 @@ def __init__(self, instance: Any, method: Optional[Callable[..., Any]] = None): if method: error_msg = f"{instance} must be initialised before {method} is called" else: - error_msg = f"instance ({instance:r}) is not initialized yet" + error_msg = f"instance ({instance:r}) is not initialised yet" super().__init__(error_msg) diff --git a/src/example_fgen_basic/pyfgen_runtime/formatting.py b/src/example_fgen_basic/pyfgen_runtime/formatting.py index ecd4e61..7245121 100644 --- a/src/example_fgen_basic/pyfgen_runtime/formatting.py +++ b/src/example_fgen_basic/pyfgen_runtime/formatting.py @@ -58,7 +58,7 @@ def to_str(instance: FinalisableWrapperBase, exposed_attributes: Iterable[str]) ------- String representation of the instance """ - if not instance.initialized: + if not instance.initialised: return f"Uninitialised {instance!r}" if not exposed_attributes: @@ -98,7 +98,7 @@ def to_pretty( indent Indent to apply to the pretty printing group """ - if not instance.initialized: + if not instance.initialised: p.text(str(instance)) return @@ -159,7 +159,7 @@ def to_html(instance: FinalisableWrapperBase, exposed_attributes: Iterable[str]) ------- HTML representation of the instance """ - if not instance.initialized: + if not instance.initialised: return str(instance) if not exposed_attributes: diff --git a/tests/unit/test_error_v.py b/tests/unit/test_error_v.py new file mode 100644 index 0000000..8f891e6 --- /dev/null +++ b/tests/unit/test_error_v.py @@ -0,0 +1,31 @@ +from example_fgen_basic.error_v import ErrorVPtrBased + + +def test_build_finalise(): + inst = ErrorVPtrBased.from_build_args(code=2, message="Hello world") + + assert inst.code == 2 + assert inst.message == "Hello world" + + inst_same_ptr = ErrorVPtrBased(inst.instance_ptr) + + assert inst_same_ptr.code == 2 + assert inst_same_ptr.message == "Hello world" + assert inst.initialised + assert inst.is_associated + + inst.finalise() + inst_same_ptr.finalise() + assert not inst.initialised + assert not inst.is_associated + + # Fun mess you can get yourself into if you use the same pointer + # and it is finalised elsewhere + assert inst_same_ptr.code != 2 + # This is true as the Python has no way of knowing + # that it has been finalised elsewhere + # (you basically need rust's borrow checker to not stuff this up) + assert inst_same_ptr.initialised + # This is how you can check for actual association + # (given this, maybe want to change how `initialised` works) + assert not inst_same_ptr.is_associated From 3d2d5b1ebb8638d1f72527ac7efa5c5ca46ddc24 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Wed, 27 Aug 2025 10:58:05 +0200 Subject: [PATCH 11/49] Add notes from talking through --- NOTES.md | 1 + src/example_fgen_basic/error_v/creation.f90 | 2 +- src/example_fgen_basic/error_v/creation_wrapper.f90 | 6 +++--- src/example_fgen_basic/error_v/error_v.f90 | 1 + src/example_fgen_basic/error_v/error_v_manager.f90 | 1 + src/example_fgen_basic/error_v/error_v_wrapper.f90 | 1 + .../fpyfgen/derived_type_manager_helpers.f90 | 2 ++ 7 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 NOTES.md diff --git a/NOTES.md b/NOTES.md new file mode 100644 index 0000000..4d76206 --- /dev/null +++ b/NOTES.md @@ -0,0 +1 @@ +- TODO: ZN to write implementation notes and why we don't use a pure pointer based solution diff --git a/src/example_fgen_basic/error_v/creation.f90 b/src/example_fgen_basic/error_v/creation.f90 index 3ad7cd8..85d43e7 100644 --- a/src/example_fgen_basic/error_v/creation.f90 +++ b/src/example_fgen_basic/error_v/creation.f90 @@ -36,7 +36,7 @@ function create_error(inv) result(err) if (mod(inv, 2) .eq. 0) then err = ErrorV(code=1, message="Even number supplied") else - err = ErrorV(code=0) + err = ErrorV(code=NO_ERROR_CODE) end if end function create_error diff --git a/src/example_fgen_basic/error_v/creation_wrapper.f90 b/src/example_fgen_basic/error_v/creation_wrapper.f90 index 51d3287..2ac97d8 100644 --- a/src/example_fgen_basic/error_v/creation_wrapper.f90 +++ b/src/example_fgen_basic/error_v/creation_wrapper.f90 @@ -39,15 +39,15 @@ subroutine create_error(inv, res_instance_index) type(ErrorV), pointer :: res + ! Use the pointer more or less like a normal instance of the derived type + res = o_create_error(inv) + ! This is the other trick for wrapping. ! We have to ensure that we have correctly associated pointers ! with the derived type instances we want to 'pass' across the Python-Fortran interface. ! Once we've done this, we can then set them more or less like normal derived types. res_instance_index = error_v_manager_get_free_instance_number() call error_v_manager_associate_pointer_with_instance(res_instance_index, res) - - ! Use the pointer more or less like a normal instance of the derived type - res = o_create_error(inv) ! Ensure that the instance index is set correctly res % instance_index = res_instance_index diff --git a/src/example_fgen_basic/error_v/error_v.f90 b/src/example_fgen_basic/error_v/error_v.f90 index 47bd788..ae47aaa 100644 --- a/src/example_fgen_basic/error_v/error_v.f90 +++ b/src/example_fgen_basic/error_v/error_v.f90 @@ -33,6 +33,7 @@ module m_error_v ! (means you can stop but also unwind errors and traceback along the way) ! TODO: think about adding trace (might be simpler than compiling with traceback) + ! type(ErrorV), allocatable, dimension(:) :: causes contains diff --git a/src/example_fgen_basic/error_v/error_v_manager.f90 b/src/example_fgen_basic/error_v/error_v_manager.f90 index 6292c4a..61e01a7 100644 --- a/src/example_fgen_basic/error_v/error_v_manager.f90 +++ b/src/example_fgen_basic/error_v/error_v_manager.f90 @@ -44,6 +44,7 @@ function get_free_instance_number() result(instance_index) end function get_free_instance_number + ! Might be a better way to do this as the pointers are a bit confusing, let's see subroutine associate_pointer_with_instance(instance_index, instance_pointer) !! Associate a pointer with the instance corresponding to the given model index !! diff --git a/src/example_fgen_basic/error_v/error_v_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_wrapper.f90 index 99ef037..16bb423 100644 --- a/src/example_fgen_basic/error_v/error_v_wrapper.f90 +++ b/src/example_fgen_basic/error_v/error_v_wrapper.f90 @@ -48,6 +48,7 @@ subroutine iget_message( & integer, intent(in) :: instance_index + ! TODO: make this variable length character(len=128), intent(out) :: message type(ErrorV), pointer :: instance diff --git a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 index 92cd3d6..a17b9ae 100644 --- a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 +++ b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 @@ -51,6 +51,8 @@ subroutine get_derived_type_free_instance_number(instance_index, n_instances, in end do + ! Should be an error or similar here + end subroutine get_derived_type_free_instance_number subroutine finalise_derived_type_instance_number(instance_index, n_instances, instance_avail, instance_array) From 7f46bb263733fd52291a22dcc94ec0427874f911 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Wed, 27 Aug 2025 13:21:33 +0200 Subject: [PATCH 12/49] Update uv.lock --- uv.lock | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/uv.lock b/uv.lock index 5099dba..5833384 100644 --- a/uv.lock +++ b/uv.lock @@ -767,6 +767,9 @@ all-dev = [ { name = "ford" }, { name = "fprettify" }, { name = "graphviz" }, + { name = "ipython", version = "8.18.1", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.10'" }, + { name = "ipython", version = "8.37.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version == '3.10.*'" }, + { name = "ipython", version = "9.4.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.11'" }, { name = "jupyterlab" }, { name = "jupytext" }, { name = "liccheck" }, @@ -786,6 +789,7 @@ all-dev = [ { name = "pymdown-extensions" }, { name = "pytest" }, { name = "pytest-cov" }, + { name = "pytest-regressions" }, { name = "ruff" }, { name = "setuptools" }, { name = "tomli" }, @@ -825,14 +829,19 @@ docs = [ { name = "ruff" }, ] tests = [ + { name = "ipython", version = "8.18.1", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.10'" }, + { name = "ipython", version = "8.37.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version == '3.10.*'" }, + { name = "ipython", version = "9.4.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.11'" }, { name = "pytest" }, { name = "pytest-cov" }, + { name = "pytest-regressions" }, ] tests-full = [ { name = "pytest-cov" }, ] tests-min = [ { name = "pytest" }, + { name = "pytest-regressions" }, ] [package.metadata] @@ -852,6 +861,7 @@ all-dev = [ { name = "ford", specifier = ">=6.0.1" }, { name = "fprettify", specifier = ">=0.3.7" }, { name = "graphviz", specifier = ">=0.21" }, + { name = "ipython", specifier = ">=8.18.1" }, { name = "jupyterlab", specifier = ">=4.4.5" }, { name = "jupytext", specifier = ">=1.17.2" }, { name = "liccheck", specifier = ">=0.9.2" }, @@ -871,6 +881,7 @@ all-dev = [ { name = "pymdown-extensions", specifier = ">=10.16.1" }, { name = "pytest", specifier = ">=8.3.4" }, { name = "pytest-cov", specifier = ">=6.0.0" }, + { name = "pytest-regressions", specifier = ">=2.8.2" }, { name = "ruff", specifier = ">=0.12.8" }, { name = "setuptools", specifier = ">=75.6.0" }, { name = "tomli", specifier = ">=2.2.1" }, @@ -910,11 +921,16 @@ docs = [ { name = "ruff", specifier = ">=0.12.8" }, ] tests = [ + { name = "ipython", specifier = ">=8.18.1" }, { name = "pytest", specifier = ">=8.3.4" }, { name = "pytest-cov", specifier = ">=6.0.0" }, + { name = "pytest-regressions", specifier = ">=2.8.2" }, ] tests-full = [{ name = "pytest-cov", specifier = ">=6.0.0" }] -tests-min = [{ name = "pytest", specifier = ">=8.3.4" }] +tests-min = [ + { name = "pytest", specifier = ">=8.3.4" }, + { name = "pytest-regressions", specifier = ">=2.8.2" }, +] [[package]] name = "exceptiongroup" @@ -3001,6 +3017,32 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/36/3b/48e79f2cd6a61dbbd4807b4ed46cb564b4fd50a76166b1c4ea5c1d9e2371/pytest_cov-6.0.0-py3-none-any.whl", hash = "sha256:eee6f1b9e61008bd34975a4d5bab25801eb31898b032dd55addc93e96fcaaa35", size = 22949, upload-time = "2024-10-29T20:13:33.215Z" }, ] +[[package]] +name = "pytest-datadir" +version = "1.8.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b4/46/db060b291999ca048edd06d6fa9ee95945d088edc38b1172c59eeb46ec45/pytest_datadir-1.8.0.tar.gz", hash = "sha256:7a15faed76cebe87cc91941dd1920a9a38eba56a09c11e9ddf1434d28a0f78eb", size = 11848, upload-time = "2025-07-30T13:52:12.518Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/8f/7a/33895863aec26ac3bb5068a73583f935680d6ab6af2a9567d409430c3ee1/pytest_datadir-1.8.0-py3-none-any.whl", hash = "sha256:5c677bc097d907ac71ca418109adc3abe34cf0bddfe6cf78aecfbabd96a15cf0", size = 6512, upload-time = "2025-07-30T13:52:11.525Z" }, +] + +[[package]] +name = "pytest-regressions" +version = "2.8.2" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pytest" }, + { name = "pytest-datadir" }, + { name = "pyyaml" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/c1/8e/e17b12ee49a9814ad86f544225331406e3bf23849a5bc83f91e119e2b72b/pytest_regressions-2.8.2.tar.gz", hash = "sha256:1d8f4767be58b9994bfa7d60271099469ad32b8ca9f9d9ceca1c1d6827156b19", size = 116642, upload-time = "2025-08-13T15:50:37.961Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b6/c9/2c3ea968876f698035b1f00dbe5cd6bff76d3e441dfb1af32111f544b6bb/pytest_regressions-2.8.2-py3-none-any.whl", hash = "sha256:a0804c1ce66d8e4d9a3c7c68f42a3d436182edca8e86565c232caeaf9e080fc2", size = 24856, upload-time = "2025-08-13T15:50:36.431Z" }, +] + [[package]] name = "python-dateutil" version = "2.9.0.post0" From 63ed6b0eb876ee35670ddb0c988a84bf1ff6362e Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Thu, 28 Aug 2025 12:38:02 +0200 Subject: [PATCH 13/49] Update implementation notes --- IMPLEMENTATION-NOTES.md | 115 ++++++++++++++++++++++++++++++++++++++++ NOTES.md | 1 - 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 IMPLEMENTATION-NOTES.md delete mode 100644 NOTES.md diff --git a/IMPLEMENTATION-NOTES.md b/IMPLEMENTATION-NOTES.md new file mode 100644 index 0000000..792bb45 --- /dev/null +++ b/IMPLEMENTATION-NOTES.md @@ -0,0 +1,115 @@ +@Marco please move these into `docs/further-background/wrapping-derived-types.md` (and do any other clean up and formatting fixes you'd like) + +Wrapping derived types is tricky. +Notably, [f2py](@Marco please add link) does not provide direct support for it. +As a result, we need to come up with our own solution. + +## Our solution + +To pass derived types back and forth across the Python-Fortran interface, +we introduce a 'manager' module for all derived types. +This manager module is responsible for managing derived type instances +that are passed across the Python-Fortran interface +and is needed because we can't pass them directly using f2py. + +The manager module has two key components: + +1. an allocatable array of instances of the derived type it manages + (@Marco note that this isn't how it is implemented now, + but this is how we will end up implementing it) +1. an allocatable array of logical (boolean) values + +The array of instances are instances which the manager owns. +It holds onto these: can instantiate them, can make them have the same values +as results from Fortran functions etc. +(@Marco I think we need to decide whether this is an array of instances +or an array of pointers to instances (although I don't think that's a thing https://fortran-lang.discourse.group/t/arrays-of-pointers/4851/6, +so doing something like this might require yet another layer of abstraction). +Array of instances means we have to do quite some data copying +and be careful about changes made on the Fortran side propagating to the Python side, +I think (although we have to test as I don't know enough about whether Fortran is pass by reference or pass by value by default), +array of pointers would mean change propagation should be more automatic. +We're going to have to define our requirements and tests quite carefully then see what works and what doesn't. +I think this is also why I introduced the 'no setters' views, +as some changes just can't be propagated back to Fortran in a 'permanent' way. +We should probably read this and do some thinking: https://stackoverflow.com/questions/4730065/why-does-a-fortran-pointer-require-a-target). + +Whenever we need to return a derived type to Python, +we follow a recipe like the below: + +1. we firstly ask the manager to give us an index (i.e. an integer) such that `logical_array(index)` is `.false.`. + The convention is that `logical_array(index)` is `.false.` means that `instance_array(index)` is available for use. +1. We set `logical_array(index)` equal to `.true.`, making clear that we are now using `instance_array(index)` +1. We set the value of `instance_array(index)` to match the the derived type that we want to return +1. We return the index value (i.e. an integer) to Python +1. The Python side just holds onto this integer +1. When we want to get attributes (i.e. values) of the derived type, + we pass the index value (i.e. an integer) of interest from Python back to Fortran +1. The manager gets the derived type at `instance_array(index)` and then can return the atribute of interest back to Python +1. When we want to set attributes (i.e. values) of the derived type, + we pass the index value (i.e. an integer) of interest and the value to set from Python back to Fortran +1. The manager gets the derived type at `instance_array(index)` and then sets the desired atribute of interest on the Fortran side +1. When we finalise an instance from Python, + we pass the index value (i.e. an integer) of interest from Python back to Fortran + and then call any finalisation routines on `instance_array(index)` on the Fortran side, + while also setting `logical_array(index)` back to `.false.`, marking `instance_array(index)` + as being available for use for another purpose + +Doing it this means that ownership is easier to manage. +Let's assume we have two Python instances backed by the same Fortran instance, +call them `PythonObjA` and `PythonObjB`. +If we finalise the Fortran instance via `PythonObjA`, then `logical_array(index)` will now be marked as `.false.`. +Then, if we try and use this instance via `PythonObjB`, +we will see that `logical_array(index)` is `.false.`, +hence we know that the object has been finalised already hence the view that `PythonObjB` has is no longer valid. +(I can see an edge case where, we finalise via `PythonObjA`, +then initialise a new object that gets the (now free) instance index +used by `PythonObjB`, so when we look again via `PythonObjB`, +we see the new object, which could be very confusing. +We should a) test this to see if we can re-create such an edge case +then b) consider a fix (maybe we need an extra array which counts how many times +this index has been initialised and finalised so we can tell if we're still +looking at the same initialisation or a new one that has happened since we last looked).) + +This solution allows us to a) only pass integers across the Python-Fortran interface +(so we can use f2py) and b) keep track of ownership. +The tradeoff is that we use more memory (because we have arrays of instances and logicals), +are slightly slower (as we have extra layers of lookup to do) +and have slow reallocation calls sometimes (when we need to increase the number of available instances dynamically). +There is no perfect solution, and we think this way strikes the right balance of +'just works' for most users while also offering access to fine-grained memory control for 'power users'. + +## Other solutions we rejected + +### Pass pointers back and forth + +Example repository: https://github.com/Nicholaswogan/f2py-with-derived-types + +Another option is to pass pointers to objects back and forth. +We tried this initially. +Where this falls over is in ownership. +Basically, the situation that doesn't work is this. + +From Python, I create an object which is backed by a Fortran derived type. +Call this `PythonObjA`. +From Python, I create another object which is backed by the same Fortran derived type instance i.e. I get a pointer to the same Fortran derived type instance. +Call this `PythonObjB`. +If I now finalise `PythonObjA` from Python, this causes the following to happen. +The pointer that was used by `PythonObjA` is now pointing to `null`. +This is fine. +However, the pointer that is being used by `PythonObjB` is now in an undefined state +(see e.g. community.intel.com/t5/Intel-Fortran-Compiler/DEALLOCATING-DATA-TYPE-POINTERS/m-p/982338#M100027 +or https://www.ibm.com/docs/en/xl-fortran-aix/16.1.0?topic=attributes-deallocate). +As a result, whenever I try to do anything with `PythonObjB`, +the result cannot be predicted and there is no way to check +(see e.g. https://stackoverflow.com/questions/72140217/can-you-test-for-nullpointers-in-fortran), +either from Python or Fortran, what the state of the pointer used by `PythonObjB` is +(it is undefined). + +This unresolvable problem is why we don't use the purely pointer-based solution +and instead go for a slightly more involved solution with a much clearer ownership model/logic. +We could do something like add a reference counter or some other solution to make this work. +This feels very complicated though. +General advice also seems to be to avoid pointers where possible +(community.intel.com/t5/Intel-Fortran-Compiler/how-to-test-if-pointer-array-is-allocated/m-p/1138643#M136486), +prefering allocatable instead, which has also helped shape our current solution. diff --git a/NOTES.md b/NOTES.md deleted file mode 100644 index 4d76206..0000000 --- a/NOTES.md +++ /dev/null @@ -1 +0,0 @@ -- TODO: ZN to write implementation notes and why we don't use a pure pointer based solution From b3aa299db06226ea7a6627f7d84b96c566fae357 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Thu, 28 Aug 2025 13:04:34 +0200 Subject: [PATCH 14/49] Get tests passing again --- Makefile | 3 +- meson.build | 1 - src/example_fgen_basic/error_v/__init__.py | 4 +- src/example_fgen_basic/error_v/creation.py | 15 +- .../error_v/creation_wrapper.f90 | 39 +--- src/example_fgen_basic/error_v/error_v.py | 111 ++---------- .../error_v/error_v_ptr_based_wrapper.f90 | 168 ------------------ .../error_v/error_v_wrapper.f90 | 47 ++++- .../pyfgen_runtime/base_finalisable.py | 139 +-------------- tests/unit/test_error_v.py | 25 ++- tests/unit/test_error_v_creation.py | 33 +--- 11 files changed, 104 insertions(+), 481 deletions(-) delete mode 100644 src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 diff --git a/Makefile b/Makefile index 9e2ce92..fb38ff5 100644 --- a/Makefile +++ b/Makefile @@ -49,8 +49,7 @@ test: ## run the tests (re-installs the package every time so you might want to # because it is looking for lines in `src` to be run, # but they're not because lines in `.venv` are run instead. # We don't have a solution to this yet. - # - # Coverage directory - needed to trick code cov to looking at the right place + uv run --no-sync python scripts/inject-srcs-into-meson-build.py uv run --no-sync python -c 'from pathlib import Path; import example_fgen_basic' || ( echo "Run make virtual-environment first" && false ) COV_DIR=$$(uv run --no-sync python -c 'from pathlib import Path; import example_fgen_basic; print(Path(example_fgen_basic.__file__).parent)'); \ uv run --no-editable --reinstall-package example-fgen-basic pytest -r a -v tests src --doctest-modules --doctest-report ndiff --cov=$$COV_DIR diff --git a/meson.build b/meson.build index 54a178e..276eef9 100644 --- a/meson.build +++ b/meson.build @@ -52,7 +52,6 @@ if pyprojectwheelbuild_enabled # Injected with `script/inject-srcs-into-meson-build.py` srcs = files( 'src/example_fgen_basic/error_v/creation_wrapper.f90', - 'src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90', 'src/example_fgen_basic/error_v/error_v_wrapper.f90', 'src/example_fgen_basic/get_wavelength_wrapper.f90', ) diff --git a/src/example_fgen_basic/error_v/__init__.py b/src/example_fgen_basic/error_v/__init__.py index ddd2b4a..4182384 100644 --- a/src/example_fgen_basic/error_v/__init__.py +++ b/src/example_fgen_basic/error_v/__init__.py @@ -2,6 +2,6 @@ Definition of an error value """ -from example_fgen_basic.error_v.error_v import ErrorV, ErrorVPtrBased +from example_fgen_basic.error_v.error_v import ErrorV -__all__ = ["ErrorV", "ErrorVPtrBased"] +__all__ = ["ErrorV"] diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py index e62c2c6..23ab07d 100644 --- a/src/example_fgen_basic/error_v/creation.py +++ b/src/example_fgen_basic/error_v/creation.py @@ -4,7 +4,7 @@ from __future__ import annotations -from example_fgen_basic.error_v.error_v import ErrorV, ErrorVPtrBased +from example_fgen_basic.error_v.error_v import ErrorV from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError try: @@ -24,16 +24,3 @@ def create_error(inv: int) -> ErrorV: error = ErrorV(instance_index) return error - - -def create_error_ptr_based(inv: int) -> ErrorVPtrBased: - """ - Create an instance of error (a wrapper around our Fortran derived type) - - Uses the pointer based logic - """ - instance_ptr = m_error_v_creation_w.create_error_ptr_based(inv) - - error = ErrorVPtrBased(instance_ptr) - - return error diff --git a/src/example_fgen_basic/error_v/creation_wrapper.f90 b/src/example_fgen_basic/error_v/creation_wrapper.f90 index 2ac97d8..a105a92 100644 --- a/src/example_fgen_basic/error_v/creation_wrapper.f90 +++ b/src/example_fgen_basic/error_v/creation_wrapper.f90 @@ -19,7 +19,7 @@ module m_error_v_creation_w implicit none private - public :: create_error, create_error_ptr_based + public :: create_error contains @@ -39,45 +39,22 @@ subroutine create_error(inv, res_instance_index) type(ErrorV), pointer :: res - ! Use the pointer more or less like a normal instance of the derived type - res = o_create_error(inv) - ! This is the other trick for wrapping. ! We have to ensure that we have correctly associated pointers ! with the derived type instances we want to 'pass' across the Python-Fortran interface. ! Once we've done this, we can then set them more or less like normal derived types. res_instance_index = error_v_manager_get_free_instance_number() + ! We have to associate res with the right index + ! before we can set it to the output of the function call + ! (in this case `o_create_error`). call error_v_manager_associate_pointer_with_instance(res_instance_index, res) - ! Ensure that the instance index is set correctly - res % instance_index = res_instance_index - - end subroutine create_error - - subroutine create_error_ptr_based(inv, res_instance_ptr) - ! Needs to be subroutine to have the created instance persist I think - ! (we can check) - ! function create_error(inv) result(res_instance_index) - integer, intent(in) :: inv - !! Input value to use to create the error - - !f2py integer(8), intent(out) :: res_instance_ptr - type(c_ptr), intent(out) :: res_instance_ptr - !! Pointer to the resulting instance - ! - ! This is the major trick for wrapping. - ! We return pointers (passed as integers) to Python rather than the instance itself. - - type(ErrorV), pointer :: res - - ! Question is: when does this get deallocated? - ! When we go out of scope? - ! If yes, that will be why we had to do this array thing. - allocate(res) + ! Use the pointer more or less like a normal instance of the derived type res = o_create_error(inv) - res_instance_ptr = c_loc(res) + ! Ensure that the instance index is set correctly + res % instance_index = res_instance_index - end subroutine create_error_ptr_based + end subroutine create_error end module m_error_v_creation_w diff --git a/src/example_fgen_basic/error_v/error_v.py b/src/example_fgen_basic/error_v/error_v.py index 9083c16..1218606 100644 --- a/src/example_fgen_basic/error_v/error_v.py +++ b/src/example_fgen_basic/error_v/error_v.py @@ -10,16 +10,13 @@ from example_fgen_basic.pyfgen_runtime.base_finalisable import ( FinalisableWrapperBase, - FinalisableWrapperBasePtrBased, check_initialised, - check_initialised_ptr_based, - execute_finalise_on_fail_ptr_based, + execute_finalise_on_fail, ) from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError try: from example_fgen_basic._lib import ( # type: ignore - m_error_v_ptr_based_w, m_error_v_w, ) except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover @@ -48,77 +45,20 @@ def exposed_attributes(self) -> tuple[str, ...]: """ return ("code", "message") - # TODO: from_build_args, from_new_connection, context manager, finalise - - @property - @check_initialised - def code(self) -> int: - """ - Error code - - Returns - ------- - : - Error code, retrieved from Fortran - """ - code: int = m_error_v_w.iget_code(instance_index=self.instance_index) - - return code - - @property - @check_initialised - def message(self) -> str: - """ - Error message - - Returns - ------- - : - Error message, retrieved from Fortran - """ - message: str = m_error_v_w.iget_message( - instance_index=self.instance_index - ).decode() - - return message - - -@define -class ErrorVPtrBased(FinalisableWrapperBasePtrBased): - """ - TODO: auto docstring e.g. "Wrapper around the Fortran :class:`ErrorV`" - - Uses the pointer-based passing logic - """ - - # Bug in Ipython pretty hence have to put this on every object? - def _repr_pretty_(self, p: Any, cycle: bool) -> None: - """ - Get pretty representation of self - - Used by IPython notebooks and other tools - """ - super()._repr_pretty_(p=p, cycle=cycle) - - @property - def exposed_attributes(self) -> tuple[str, ...]: - """ - Attributes exposed by this wrapper - """ - return ("code", "message") - + # TODO: context manager @classmethod - def from_new_connection(cls) -> ErrorVPtrBased: + def from_new_connection(cls) -> ErrorV: """ Initialise from a new connection The user is responsible for releasing this connection using :attr:`~finalise` when it is no longer needed. - Alternatively a :obj:`~AtmosphereToOceanCarbonFluxCalculatorContext` + Alternatively an [ErrorVContext][] can be used to handle the finalisation using a context manager. Returns ------- + : A new instance with a unique instance index Raises @@ -128,26 +68,24 @@ def from_new_connection(cls) -> ErrorVPtrBased: This could occur if too many instances are allocated at any one time """ - instance_ptr = m_error_v_ptr_based_w.get_instance_ptr() - # TODO: result type handling here + instance_ptr: int = m_error_v_w.get_free_instance_number() return cls(instance_ptr) - # TODO: from_build_args, from_new_connection, context manager, finalise @classmethod def from_build_args( cls, code: int, message: str = "", - ) -> ErrorVPtrBased: + ) -> ErrorV: """ Build the class (including connecting to Fortran) """ out = cls.from_new_connection() # TODO: remove or update this construct when we have result types - execute_finalise_on_fail_ptr_based( + execute_finalise_on_fail( out, - m_error_v_ptr_based_w.instance_build, + m_error_v_w.instance_build, code=code, message=message, ) @@ -159,28 +97,11 @@ def finalise(self) -> None: """ Close the connection with the Fortran module """ - m_error_v_ptr_based_w.instance_finalise(self.instance_ptr) - self._uninitialise_instance_ptr() - - @property - def is_associated(self) -> bool: - """ - Check whether `self`'s pointer is associated on the Fortran side - - Returns - ------- - : - Whether `self`'s pointer is associated on the Fortran side - """ - if self.instance_ptr is None: - return False - - res: bool = bool(m_error_v_ptr_based_w.is_associated(self.instance_ptr)) # type: ignore - - return res + m_error_v_w.instance_finalise(self.instance_index) + self._uninitialise_instance_index() @property - @check_initialised_ptr_based + @check_initialised def code(self) -> int: """ Error code @@ -190,12 +111,12 @@ def code(self) -> int: : Error code, retrieved from Fortran """ - code: int = m_error_v_ptr_based_w.iget_code(instance_ptr=self.instance_ptr) + code: int = m_error_v_w.iget_code(instance_index=self.instance_index) return code @property - @check_initialised_ptr_based + @check_initialised def message(self) -> str: """ Error message @@ -205,8 +126,8 @@ def message(self) -> str: : Error message, retrieved from Fortran """ - message: str = m_error_v_ptr_based_w.iget_message( - instance_ptr=self.instance_ptr + message: str = m_error_v_w.iget_message( + instance_index=self.instance_index ).decode() return message diff --git a/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 deleted file mode 100644 index 2f59218..0000000 --- a/src/example_fgen_basic/error_v/error_v_ptr_based_wrapper.f90 +++ /dev/null @@ -1,168 +0,0 @@ -!> Wrapper for interfacing `m_error_v` with Python, using pointer only (no instance array) -!> -!> Written by hand here. -!> Generation to be automated in future (including docstrings of some sort). -module m_error_v_ptr_based_w - - use iso_c_binding, only: c_f_pointer, c_loc, c_null_ptr, c_ptr - - use m_error_v, only: ErrorV - - implicit none - private - - public :: get_instance_ptr, instance_build, instance_finalise, is_associated, & - iget_code, iget_message - -contains - - subroutine get_instance_ptr(res_instance_ptr) - !> Get a pointer to a new instance - ! - ! Needs to be subroutine to have the created instance persist I think - ! (we can check) - ! function create_error(inv) result(res_instance_index) - - !f2py integer(8), intent(out) :: res_instance_ptr - type(c_ptr), intent(out) :: res_instance_ptr - !! Pointer to the resulting instance - ! - ! This is the major trick for wrapping. - ! We return pointers (passed as integers) to Python rather than the instance itself. - - type(ErrorV), pointer :: res - ! Question is: when does this get deallocated? - ! When we go out of scope? - ! If yes, that will be why we had to do this array thing. - allocate(res) - - res_instance_ptr = c_loc(res) - - end subroutine get_instance_ptr - - subroutine instance_build(instance_ptr, code, message) - !> Build an instance - - !f2py integer(8), intent(in) :: instance_ptr - type(c_ptr), intent(in) :: instance_ptr - !! Pointer to the instance - ! - ! This is the major trick for wrapping. - ! We pass pointers (passed as integers) to Python rather than the instance itself. - - integer, intent(in) :: code - character(len=*), optional, intent(in) :: message - - type(ErrorV), pointer :: inst - - call c_f_pointer(instance_ptr, inst) - - call inst % build(code, message) - - end subroutine instance_build - - subroutine instance_finalise(instance_ptr) - !> Finalise an instance - - !f2py integer(8), intent(inout) :: instance_ptr - type(c_ptr), intent(inout) :: instance_ptr - !! Pointer to the instance - ! - ! This is the major trick for wrapping. - ! We pass pointers (passed as integers) to Python rather than the instance itself. - - type(ErrorV), pointer :: inst - - call c_f_pointer(instance_ptr, inst) - - ! This may be why we used the array approach. - ! The issue here is that, if you call this method twice, - ! there is no way to work out that you're the 'second caller'. - ! When the first call calls `deallocate(inst)`, - ! this puts any other pointers to the instance in an undefined status - ! (https://www.ibm.com/docs/en/xl-fortran-aix/16.1.0?topic=attributes-deallocate). - ! The result of calling associated on an undefined pointer - ! can be anything (https://stackoverflow.com/questions/72140217/can-you-test-for-nullpointers-in-fortran), - ! i.e. there is no way to tell that someone else - ! has already called finalise before you have. - ! This also explains the undefined status issue nicely: - ! community.intel.com/t5/Intel-Fortran-Compiler/DEALLOCATING-DATA-TYPE-POINTERS/m-p/982338#M100027 - ! - ! We'd have to introduce some reference counter to make this work I think. - ! Probably better advice for now, don't share pointer values - ! on the Python side, you have to be super careful about uninitialising if you do. - ! Avoiding pointers and using allocatable instead - ! was probably the other reason we did it how we did - ! community.intel.com/t5/Intel-Fortran-Compiler/how-to-test-if-pointer-array-is-allocated/m-p/1138643#M136486. - if (associated(inst)) then - call inst % finalise() - deallocate(inst) - end if - - end subroutine instance_finalise - - subroutine is_associated(instance_ptr, res) - !> Check if a pointer is associated with an instance - - !f2py integer(8), intent(in) :: instance_ptr - type(c_ptr), intent(in) :: instance_ptr - !! Pointer to the instance - ! - ! This is the major trick for wrapping. - ! We pass pointers (passed as integers) to Python rather than the instance itself. - - logical, intent(out) :: res - !! Whether `instance_ptr` is associated or not - - type(ErrorV), pointer :: inst - - call c_f_pointer(instance_ptr, inst) - - print *, instance_ptr - print *, inst - res = associated(inst) - print *, res - - end subroutine is_associated - - ! Full set of wrapping strategies to pass different types in e.g. - ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 - ! (we will do a full re-write of the code which generates this, - ! but the strategies will probably stay as they are) - subroutine iget_code( & - instance_ptr, & - code & - ) - - !f2py integer(8), intent(in) :: instance_ptr - type(c_ptr), intent(in) :: instance_ptr - - integer, intent(out) :: code - - type(ErrorV), pointer :: instance - - call c_f_pointer(instance_ptr, instance) - - code = instance % code - - end subroutine iget_code - - subroutine iget_message( & - instance_ptr, & - message & - ) - - !f2py integer(8), intent(in) :: instance_ptr - type(c_ptr), intent(in) :: instance_ptr - - character(len=128), intent(out) :: message - - type(ErrorV), pointer :: instance - - call c_f_pointer(instance_ptr, instance) - - message = instance % message - - end subroutine iget_message - -end module m_error_v_ptr_based_w diff --git a/src/example_fgen_basic/error_v/error_v_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_wrapper.f90 index 16bb423..dacc5d2 100644 --- a/src/example_fgen_basic/error_v/error_v_wrapper.f90 +++ b/src/example_fgen_basic/error_v/error_v_wrapper.f90 @@ -9,17 +9,60 @@ module m_error_v_w ! The manager module, which makes this all work use m_error_v_manager, only: & - get_free_instance_number, & + error_v_manager_get_free_instance_number => get_free_instance_number, & + error_v_manager_finalise_instance => finalise_instance, & error_v_manager_associate_pointer_with_instance => associate_pointer_with_instance ! TODO: build and finalisation implicit none private - public :: get_free_instance_number, iget_code, iget_message + public :: get_free_instance_number, instance_build, instance_finalise, & + iget_code, iget_message contains + function get_free_instance_number() result(instance_index) + + integer :: instance_index + + instance_index = error_v_manager_get_free_instance_number() + + end function get_free_instance_number + + subroutine instance_build(instance_index, code, message) + !> Build an instance + + integer, intent(in) :: instance_index + !! Instance index + ! + ! This is the major trick for wrapping. + ! We pass instance indexes (integers) to Python rather than the instance itself. + + integer, intent(in) :: code + character(len=*), optional, intent(in) :: message + + type(ErrorV), pointer :: instance + + call error_v_manager_associate_pointer_with_instance(instance_index, instance) + + call instance % build(code, message) + + end subroutine instance_build + + subroutine instance_finalise(instance_index) + !> Finalise an instance + + integer, intent(in) :: instance_index + !! Instance index + ! + ! This is the major trick for wrapping. + ! We pass instance indexes (integers) to Python rather than the instance itself. + + call error_v_manager_finalise_instance(instance_index) + + end subroutine instance_finalise + ! Full set of wrapping strategies to pass different types in e.g. ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 ! (we will do a full re-write of the code which generates this, diff --git a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py index 9dc45c2..dab17f0 100644 --- a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py +++ b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py @@ -165,140 +165,10 @@ def checked( return checked # type: ignore -@define -class FinalisableWrapperBasePtrBased(ABC): - """ - Base class for Fortran derived type wrappers using the pointer-based passing - """ - - instance_ptr: int | None = None - """ - Pointer to Fortran instance (technically a C pointer) - """ - - def __str__(self) -> str: - """ - Get string representation of self - """ - return to_str( - self, - self.exposed_attributes, - ) - - def _repr_pretty_(self, p: Any, cycle: bool) -> None: - """ - Get pretty representation of self - - Used by IPython notebooks and other tools - """ - to_pretty( - self, - self.exposed_attributes, - p=p, - cycle=cycle, - ) - - def _repr_html_(self) -> str: - """ - Get html representation of self - - Used by IPython notebooks and other tools - """ - return to_html( - self, - self.exposed_attributes, - ) - - @property - def initialised(self) -> bool: - """ - Is the instance initialised, i.e. connected to a Fortran instance? - """ - return self.instance_ptr is not None - - @property - @abstractmethod - def exposed_attributes(self) -> tuple[str, ...]: - """ - Attributes exposed by this wrapper - """ - ... - - @classmethod - @abstractmethod - def from_new_connection(cls) -> FinalisableWrapperBase: - """ - Initialise by establishing a new connection with the Fortran module - - This requests a new model index from the Fortran module and then - initialises a class instance. - - Returns - ------- - : - New class instance - """ - ... - - @abstractmethod - def finalise(self) -> None: - """ - Finalise the Fortran instance and set self back to being uninitialised - - This method resets `self.instance_ptr` back to `None`. - - Should be decorated with :func:`check_initialised` - """ - # call to Fortran module goes here when implementing - self._uninitialise_instance_ptr() - - def _uninitialise_instance_ptr(self) -> None: - self.instance_ptr = None - - -WrapperPtrBased = TypeVar("WrapperPtrBased", bound=FinalisableWrapperBasePtrBased) - - -def check_initialised_ptr_based( - method: Callable[Concatenate[WrapperPtrBased, P], T], -) -> Callable[Concatenate[WrapperPtrBased, P], T]: - """ - Check that the wrapper object has been initialised before executing the method - - Parameters - ---------- - method - Method to wrap - - Returns - ------- - : - Wrapped method - - Raises - ------ - InitialisationError - Wrapper is not initialised - """ - - @wraps(method) - def checked( - ref: WrapperPtrBased, - *args: P.args, - **kwargs: P.kwargs, - ) -> Any: - if not ref.initialised: - raise NotInitialisedError(ref, method) - - return method(ref, *args, **kwargs) - - return checked # type: ignore - - # Thank you for type hints info # https://adamj.eu/tech/2021/05/11/python-type-hints-args-and-kwargs/ -def execute_finalise_on_fail_ptr_based( - inst: FinalisableWrapperBasePtrBased, +def execute_finalise_on_fail( + inst: FinalisableWrapperBase, func_to_try: Callable[Concatenate[int, P], T], *args: P.args, **kwargs: P.kwargs, @@ -310,6 +180,9 @@ def execute_finalise_on_fail_ptr_based( clean way of ensuring that any Fortran is freed up in the event of an initialisation failure for any reason + @Marco note: I'm not sure this is a very good pattern, + we may get rid of it. + Parameters ---------- inst @@ -337,7 +210,7 @@ def execute_finalise_on_fail_ptr_based( exception is raised, `inst.finalise()` is called. """ try: - return func_to_try(inst.instance_ptr, *args, **kwargs) + return func_to_try(inst.instance_index, *args, **kwargs) except RuntimeError: # finalise the instance before raising inst.finalise() diff --git a/tests/unit/test_error_v.py b/tests/unit/test_error_v.py index 8f891e6..1ff01b6 100644 --- a/tests/unit/test_error_v.py +++ b/tests/unit/test_error_v.py @@ -1,13 +1,32 @@ -from example_fgen_basic.error_v import ErrorVPtrBased +""" +Tests of `example_fgen_basic.error_v` +""" + +import pytest + +from example_fgen_basic.error_v import ErrorV def test_build_finalise(): - inst = ErrorVPtrBased.from_build_args(code=2, message="Hello world") + inst = ErrorV.from_build_args(code=2, message="Hello world") + + assert inst.code == 2 + assert inst.message == "Hello world" + + assert inst.initialised + + inst.finalise() + assert not inst.initialised + + +def test_build_finalise_multiple_instances_same_index(): + pytest.skip("TODO: turn back on") + inst = ErrorV.from_build_args(code=2, message="Hello world") assert inst.code == 2 assert inst.message == "Hello world" - inst_same_ptr = ErrorVPtrBased(inst.instance_ptr) + inst_same_ptr = ErrorV(inst.instance_ptr) assert inst_same_ptr.code == 2 assert inst_same_ptr.message == "Hello world" diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py index a23df1b..cdda469 100644 --- a/tests/unit/test_error_v_creation.py +++ b/tests/unit/test_error_v_creation.py @@ -9,8 +9,8 @@ import pytest from IPython.lib.pretty import pretty -from example_fgen_basic.error_v import ErrorV, ErrorVPtrBased -from example_fgen_basic.error_v.creation import create_error, create_error_ptr_based +from example_fgen_basic.error_v import ErrorV +from example_fgen_basic.error_v.creation import create_error def test_create_error_odd(): @@ -42,6 +42,7 @@ def test_create_error_negative(): def test_error_too_many_instances(): + # @Marco we will fix this when we introduce a result type in a future step pytest.skip("Causes segfault right now") # - if we create more errors than we have available, we don't segfault. # Instead, we should get an error back. @@ -78,31 +79,3 @@ def test_error_html(file_regression): res = create_error(1.0) file_regression.check(res._repr_html_(), extension=".html") - - -def test_create_error_even_ptr_based(): - res = create_error_ptr_based(2.0) - - assert isinstance(res, ErrorVPtrBased) - - assert res.code != 0 - assert res.code == 1 - assert res.message == "Even number supplied" - - -def test_error_ptr_based_str(file_regression): - res = create_error_ptr_based(1.0) - - file_regression.check(str(res)) - - -def test_error_ptr_based_pprint(file_regression): - res = create_error_ptr_based(1.0) - - file_regression.check(pretty(res)) - - -def test_error_ptr_based_html(file_regression): - res = create_error_ptr_based(1.0) - - file_regression.check(res._repr_html_(), extension=".html") From 30ba7748e966dfdc1dda0eb05debd64518ef5f84 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Thu, 28 Aug 2025 13:18:55 +0200 Subject: [PATCH 15/49] Fix up tests except for actually failing one --- .../fpyfgen/derived_type_manager_helpers.f90 | 2 +- tests/unit/test_error_v.py | 54 ++++++++++++------- tests/unit/test_error_v_creation.py | 17 ++++-- .../test_error_html.html | 2 +- .../test_error_pprint.txt | 2 +- .../test_error_v_creation/test_error_str.txt | 2 +- 6 files changed, 54 insertions(+), 25 deletions(-) diff --git a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 index a17b9ae..137c963 100644 --- a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 +++ b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 @@ -33,7 +33,7 @@ subroutine get_derived_type_free_instance_number(instance_index, n_instances, in class(BaseFinalisable), dimension(n_instances), intent(inout) :: instance_array !! Array of instances - integer :: i + integer :: i = 1 ! Default if no available models are found instance_index = invalid_instance_index diff --git a/tests/unit/test_error_v.py b/tests/unit/test_error_v.py index 1ff01b6..3865a42 100644 --- a/tests/unit/test_error_v.py +++ b/tests/unit/test_error_v.py @@ -20,31 +20,49 @@ def test_build_finalise(): def test_build_finalise_multiple_instances_same_index(): - pytest.skip("TODO: turn back on") inst = ErrorV.from_build_args(code=2, message="Hello world") + original_instance_index = inst.instance_index + assert inst.code == 2 assert inst.message == "Hello world" - inst_same_ptr = ErrorV(inst.instance_ptr) + inst_same_index = ErrorV(inst.instance_index) - assert inst_same_ptr.code == 2 - assert inst_same_ptr.message == "Hello world" + assert inst_same_index.code == 2 + assert inst_same_index.message == "Hello world" assert inst.initialised - assert inst.is_associated inst.finalise() - inst_same_ptr.finalise() + # # Currently this causes a hard stop. + # # That's the right behaviour. + # # We will make it not be a hard fail when we switch to result types. + # inst_same_index.finalise() assert not inst.initialised - assert not inst.is_associated - - # Fun mess you can get yourself into if you use the same pointer - # and it is finalised elsewhere - assert inst_same_ptr.code != 2 - # This is true as the Python has no way of knowing - # that it has been finalised elsewhere - # (you basically need rust's borrow checker to not stuff this up) - assert inst_same_ptr.initialised - # This is how you can check for actual association - # (given this, maybe want to change how `initialised` works) - assert not inst_same_ptr.is_associated + + ### Problem 1 ### + # # With the current implementation, + # # finalising via `inst` does not cause `inst_same_index` to also be finalised + # assert not inst_same_index.initialised + + inst_new = ErrorV.from_build_args(code=3, message="Didn't expect this") + # New instance uses the newly freed index. + assert inst_new.instance_index == original_instance_index + # Which means the new instance and the instance + # which was initialised previously now have the same instance index + assert inst_new.instance_index == inst_same_index.instance_index + + ### Problem 2 ### + # So, if we look at `inst_same_index`'s attribute values, we get a surprise + # The code isn't 2 as was set above. + # Instead, the value has changed 'in the background' i.e. our view is 'stale'. + assert inst_same_index.code == 3 + assert inst_same_index.message == "Didn't expect this" + + # Something like this is what we actually want to happen + # (if we try to access via inst_same_index again, + # we get told that our view is out of date). + # Could be warning or error, not sure what makes more sense... + with pytest.raises(StaleViewError): + inst_same_index.code + inst_same_index.message diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py index cdda469..87161ca 100644 --- a/tests/unit/test_error_v_creation.py +++ b/tests/unit/test_error_v_creation.py @@ -6,6 +6,8 @@ (deliberately, just to see how it goes). """ +import re + import pytest from IPython.lib.pretty import pretty @@ -54,6 +56,9 @@ def test_error_too_many_instances(): create_error(1) +@pytest.mark.xfail( + reason="Not implemented yet - do in a future PR once we have a result type" +) def test_increase_number_of_instances(): raise NotImplementedError # - Make 4096 instances @@ -66,16 +71,22 @@ def test_increase_number_of_instances(): def test_error_str(file_regression): res = create_error(1.0) - file_regression.check(str(res)) + # Don't worry about the value of instance_index + res_check = re.sub(r"instance_index=\d*", "instance_index=n", str(res)) + file_regression.check(res_check) def test_error_pprint(file_regression): res = create_error(1.0) - file_regression.check(pretty(res)) + # Don't worry about the value of instance_index + res_check = re.sub(r"instance_index=\d*", "instance_index=n", pretty(res)) + file_regression.check(res_check) def test_error_html(file_regression): res = create_error(1.0) - file_regression.check(res._repr_html_(), extension=".html") + # Don't worry about the value of instance_index + res_check = re.sub(r"instance_index=\d*", "instance_index=n", res._repr_html_()) + file_regression.check(res_check, extension=".html") diff --git a/tests/unit/test_error_v_creation/test_error_html.html b/tests/unit/test_error_v_creation/test_error_html.html index d499603..09d6855 100644 --- a/tests/unit/test_error_v_creation/test_error_html.html +++ b/tests/unit/test_error_v_creation/test_error_html.html @@ -31,7 +31,7 @@
ErrorV
-
instance_index=6
+
instance_index=n
diff --git a/tests/unit/test_error_v_creation/test_error_pprint.txt b/tests/unit/test_error_v_creation/test_error_pprint.txt index 502a0db..c74e0c2 100644 --- a/tests/unit/test_error_v_creation/test_error_pprint.txt +++ b/tests/unit/test_error_v_creation/test_error_pprint.txt @@ -1 +1 @@ -ErrorV(instance_index=5, code=0, message=) +ErrorV(instance_index=n, code=0, message=) diff --git a/tests/unit/test_error_v_creation/test_error_str.txt b/tests/unit/test_error_v_creation/test_error_str.txt index 9cc1073..c74e0c2 100644 --- a/tests/unit/test_error_v_creation/test_error_str.txt +++ b/tests/unit/test_error_v_creation/test_error_str.txt @@ -1 +1 @@ -ErrorV(instance_index=4, code=0, message=) +ErrorV(instance_index=n, code=0, message=) From 0bb09e49eb267102f1ff0e371e8564ac61de4a26 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Thu, 28 Aug 2025 13:20:19 +0200 Subject: [PATCH 16/49] Remove unused files --- tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt | 1 - tests/unit/test_error_v_creation/test_error_ptr_based_str.txt | 1 - 2 files changed, 2 deletions(-) delete mode 100644 tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt delete mode 100644 tests/unit/test_error_v_creation/test_error_ptr_based_str.txt diff --git a/tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt b/tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt deleted file mode 100644 index 6f373c0..0000000 --- a/tests/unit/test_error_v_creation/test_error_ptr_based_pprint.txt +++ /dev/null @@ -1 +0,0 @@ -ErrorVPtrBased(instance_ptr=105553174233376, code=0, message=) diff --git a/tests/unit/test_error_v_creation/test_error_ptr_based_str.txt b/tests/unit/test_error_v_creation/test_error_ptr_based_str.txt deleted file mode 100644 index 21467d4..0000000 --- a/tests/unit/test_error_v_creation/test_error_ptr_based_str.txt +++ /dev/null @@ -1 +0,0 @@ -ErrorVPtrBased(instance_ptr=105553174233232, code=0, message=) From 8006ddafee7699f3b4f1a129f4cfab78de8b2b32 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 29 Aug 2025 12:09:25 +0200 Subject: [PATCH 17/49] Update implementation notes --- IMPLEMENTATION-NOTES.md | 183 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 176 insertions(+), 7 deletions(-) diff --git a/IMPLEMENTATION-NOTES.md b/IMPLEMENTATION-NOTES.md index 792bb45..e09dceb 100644 --- a/IMPLEMENTATION-NOTES.md +++ b/IMPLEMENTATION-NOTES.md @@ -1,11 +1,184 @@ @Marco please move these into `docs/further-background/wrapping-derived-types.md` (and do any other clean up and formatting fixes you'd like) -Wrapping derived types is tricky. -Notably, [f2py](@Marco please add link) does not provide direct support for it. +## What is the goal? + +The goal is to be able to run MAGICC, a model written in Fortran, from Python. +This means we need to be able to instantiate MAGICC's inputs in memory in Python, +pass them to Fortran to solve the model and get them back as results in Python. + +Our data is not easily represented as primitive types (floats, ints, strings, arrays) +because we want to have more robust data handling, e.g. attaching units to arrays. +As a result, we need to pass objects to Fortran and return Fortran derived types to Python. +It turns out that wrapping derived types is tricky. +Notably, [f2py](https://numpy.org/doc/stable/f2py/) +does not provide direct support for it. As a result, we need to come up with our own solution. ## Our solution +Our solution is based on a key simplifying assumption. +Once we have passed data across the Python-Fortran interface, +there is no way to modify it again from the other side of the interface. +In other words, our wrappers are not views, +instead they are independent instantiations of the same (or as similar as possible) data models. + +For example, if I have an object in Python +and I pass this to a wrapped Fortran function which alters some attribute of this object, +that modification will only happen on the Fortran side, +the original Python object will remain unchanged +(as a note, to see the result, we must return a new Python object from the Fortran wrapper). + +This assumption makes ownership and memory management clear. +We do not need to keep instances around as views +and worry about consistency across the Python-Fortran interface. +Instead, we simply pass data back and forth, +and the normal rules of data consistency within each programming language apply. + +To actually pass derived types back and forth across the Python-Fortran interface, +we introduce a 'manager' module for all derived types. + +The manager module has two key components: + +1. an allocatable array of instances of the derived type it manages, + call this `instance_array`. + The array of instances are instances which the manager owns. + In practice, they are essentially temporary variables. +1. an allocatable array of logical (boolean) values, + call this `available_array`. + The convention is that, if `available_array(i)`, where `i` is an integer, + is `.true.` then the instance at `instance_array(i)` is available for the manager to use, + otherwise the manager assumes that the instance is already being used for some purpose + and therefore cannot be used for whatever operation is currently being performed. + +This setup allows us to effectively pass derived types back and forth between Python and Fortran. + +Whenever we need to return a derived type to Python, we: + +[TODO think about retrieving multiple derived types at once] + +1. get the derived type from whatever Fortran function or subroutine created it, + call this `derived_type_original` +1. find an index, `idx`, in `available_array` such that `available_array(idx)` is `.true.` +1. set `instance_array(idx)` equal to `derived_type_original` +1. we return `idx` to Python + - `idx` is an integer, so we can return this easily to Python using `f2py` +1. we then create a Python object with an API that mirrors `derived_type_original` + using the class method `from_instance_index`. + This class method is [TODO or will be] auto-generated via `pyfgen` + and handles retrieval of all the attribute values of `derived_type_original` + from Fortran and sets them on the Python object that is being instantiated + - we can do this as, if you dig down deep enough, all attributes eventually + become primitive types which can be passed back and forth using `f2py`, + it can just be that multiple levels of recursion are needed + if you have derived types that themselves have derived type attributes +1. we then call the manager [TODO I think this will end up being wrapper, we can tighten the language later] + module's `finalise_instance_index` function to free the (temporary) instance + that was used by the manager + - this instance is no longer needed because all the data has been transferred to Python +1. we end up with a Python instance that has the result + and no extra/leftover memory footprint in Fortran + (and leave Fortran to decide whether to clean up `derived_type_original` or not) + +Whenever we need to pass a derived type to Fortran, we: + +[TODO think about passing multiple derived types at once] + +1. call the manager [TODO I think this will end up being wrapper, we can tighten the language later] + module's `get_free_instance_index` function to get an available index to use for the passing +1. call the manager [TODO I think this will end up being wrapper, we can tighten the language later] + module's `build_instance` function with the index we just received + plus all of the Python object's attribute values + - on the Fortran side, there is now an instantiated derived type, ready for use +1. call the wrapped Fortran function of interest, + except we pass the instance index instead of the derived type +1. on the Fortran side, retrieve the instantiated index from the manager module + and use this to call the Fortran function/subroutine of interest +1. return the result from Fortran back to Python +1. call the manager [TODO I think this will end up being wrapper, we can tighten the language later] + module's `finalise_instance_index` function to free the (temporary) instance + that was used to pass the instance in the first place + - this instance is no longer needed because all the data has been transferred and used by Fortran +1. we end up with the result of the Fortran callable back in Python + and no extra/leftover memory footprint in Fortran from the instance created by the manager module + +## Further background + +We initially started this project and took quite a different route. +The reason was that we were actually solving a different problem. +What we were trying to do was to provide views into underlying Fortran instances. +For example, we wanted to enable the following: + +```python +>>> from some_fortran_wrapper import SomeWrappedFortranDerivedType + + +>>> inst = SomeWrappedFortranDerivedType(value1=2, value2="hi") +>>> inst2 = inst +>>> inst.value1 = 5 +>>> # Updating the view via `inst` also affects `inst2` +>>> inst2.value1 +5 +``` + +Supporting views like this introduces a whole bunch of headaches, +mainly due to consistency and memory management. + +A first headache is consistency. +Consider the following, which is a common gotcha with numpy + +```python +>>> import numpy as np +>>> +>>> a = np.array([1.2, 2.2, 2.5]) +>>> b = a +>>> a[2] = 0.0 +>>> # b has been updated too - many users don't expect this +>>> b +array([1.2, 2.2, 0. ]) +``` + +The second is memory management. +For example, in the example above, if I delete variable `a`, +what should variable `b` become? + +With numpy, it turns out that the answer is that `b` is unaffected + +```python +>>> del a +>>> a +Traceback (most recent call last): + File "", line 1, in +NameError: name 'a' is not defined +>>> b +array([1.2, 2.2, 0. ]) +``` + +However, we would argue that this is not the only possibility. +It could also be that `b` should become undefined, +as the underlying array it views has been deleted. +Doing it like this must also be very complicated for numpy, +as they need to keep track of how many references +there are to the array underlying the Python variables +to know whether to actually free the memory or not. + +We don't want to solve these headaches, +which is why our solution does not support views, +instead only supporting the passing of data across the Python-Fortran interface +(which ensures that ownership is clear at all times +and normal Python rules apply in Python +(which doesn't mean there aren't gotchas, just that we won't introduce any new gotchas)). + +## Other solutions we rejected + +### Provide views rather than passing data + +Note: this section was never properly finished. +Once we started trying to write it, +we realised how hard it would be to avoid weird edge cases +so we stopped and changed to [our current solution][Our solution] +(@Marco please check that this internal cross-reference works +once the docs are built). + To pass derived types back and forth across the Python-Fortran interface, we introduce a 'manager' module for all derived types. This manager module is responsible for managing derived type instances @@ -15,14 +188,12 @@ and is needed because we can't pass them directly using f2py. The manager module has two key components: 1. an allocatable array of instances of the derived type it manages - (@Marco note that this isn't how it is implemented now, - but this is how we will end up implementing it) 1. an allocatable array of logical (boolean) values The array of instances are instances which the manager owns. It holds onto these: can instantiate them, can make them have the same values as results from Fortran functions etc. -(@Marco I think we need to decide whether this is an array of instances +(I think we need to decide whether this is an array of instances or an array of pointers to instances (although I don't think that's a thing https://fortran-lang.discourse.group/t/arrays-of-pointers/4851/6, so doing something like this might require yet another layer of abstraction). Array of instances means we have to do quite some data copying @@ -79,8 +250,6 @@ and have slow reallocation calls sometimes (when we need to increase the number There is no perfect solution, and we think this way strikes the right balance of 'just works' for most users while also offering access to fine-grained memory control for 'power users'. -## Other solutions we rejected - ### Pass pointers back and forth Example repository: https://github.com/Nicholaswogan/f2py-with-derived-types From 1069a0c0bee668b4d316119aa35b44ee3b56e751 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 29 Aug 2025 12:16:58 +0200 Subject: [PATCH 18/49] Add first set of tests --- tests/unit/test_error_v.py | 68 -------------------------- tests/unit/test_error_v_creation.py | 76 +++++++++-------------------- 2 files changed, 24 insertions(+), 120 deletions(-) delete mode 100644 tests/unit/test_error_v.py diff --git a/tests/unit/test_error_v.py b/tests/unit/test_error_v.py deleted file mode 100644 index 3865a42..0000000 --- a/tests/unit/test_error_v.py +++ /dev/null @@ -1,68 +0,0 @@ -""" -Tests of `example_fgen_basic.error_v` -""" - -import pytest - -from example_fgen_basic.error_v import ErrorV - - -def test_build_finalise(): - inst = ErrorV.from_build_args(code=2, message="Hello world") - - assert inst.code == 2 - assert inst.message == "Hello world" - - assert inst.initialised - - inst.finalise() - assert not inst.initialised - - -def test_build_finalise_multiple_instances_same_index(): - inst = ErrorV.from_build_args(code=2, message="Hello world") - - original_instance_index = inst.instance_index - - assert inst.code == 2 - assert inst.message == "Hello world" - - inst_same_index = ErrorV(inst.instance_index) - - assert inst_same_index.code == 2 - assert inst_same_index.message == "Hello world" - assert inst.initialised - - inst.finalise() - # # Currently this causes a hard stop. - # # That's the right behaviour. - # # We will make it not be a hard fail when we switch to result types. - # inst_same_index.finalise() - assert not inst.initialised - - ### Problem 1 ### - # # With the current implementation, - # # finalising via `inst` does not cause `inst_same_index` to also be finalised - # assert not inst_same_index.initialised - - inst_new = ErrorV.from_build_args(code=3, message="Didn't expect this") - # New instance uses the newly freed index. - assert inst_new.instance_index == original_instance_index - # Which means the new instance and the instance - # which was initialised previously now have the same instance index - assert inst_new.instance_index == inst_same_index.instance_index - - ### Problem 2 ### - # So, if we look at `inst_same_index`'s attribute values, we get a surprise - # The code isn't 2 as was set above. - # Instead, the value has changed 'in the background' i.e. our view is 'stale'. - assert inst_same_index.code == 3 - assert inst_same_index.message == "Didn't expect this" - - # Something like this is what we actually want to happen - # (if we try to access via inst_same_index again, - # we get told that our view is out of date). - # Could be warning or error, not sure what makes more sense... - with pytest.raises(StaleViewError): - inst_same_index.code - inst_same_index.message diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py index 87161ca..3303090 100644 --- a/tests/unit/test_error_v_creation.py +++ b/tests/unit/test_error_v_creation.py @@ -1,18 +1,16 @@ """ Tests of `example_fgen_basic.error_v.creation` - -Note that this is the only test of the Fortran code. -I haven't written unit tests for the Fortran directly -(deliberately, just to see how it goes). """ -import re - -import pytest -from IPython.lib.pretty import pretty +import numpy as np from example_fgen_basic.error_v import ErrorV -from example_fgen_basic.error_v.creation import create_error +from example_fgen_basic.error_v.creation import create_error, create_errors + +# Tests to write: +# - passing derived types to Fortran (new test module) +# - retrieving multiple derived type instances from Fortran +# (basically checking the manager's auto-resizing of the number of instances) def test_create_error_odd(): @@ -43,50 +41,24 @@ def test_create_error_negative(): assert res.message == "Negative number supplied" -def test_error_too_many_instances(): - # @Marco we will fix this when we introduce a result type in a future step - pytest.skip("Causes segfault right now") - # - if we create more errors than we have available, we don't segfault. - # Instead, we should get an error back. - # That error should just use the instance ID of the last available array index - # (it is ok to overwrite an already used error to avoid a complete failure, - # but we should probably include that we did this in the error message). - # TODO: expect error here - for _ in range(4097): +def test_create_error_lots_of_repeated_calls(): + # We should be able to just keep calling `create_error` + # without hitting segfaults or other weirdness. + # This is basically testing that we're freeing the temporary + # Fortran derived types correctly + # (and sort of a speed test, this shouldn't be noticeably slow) + # hence we may move this test somewhere more generic at some point. + for _ in range(1e5): create_error(1) -@pytest.mark.xfail( - reason="Not implemented yet - do in a future PR once we have a result type" -) -def test_increase_number_of_instances(): - raise NotImplementedError - # - Make 4096 instances - # - show that making one more raises an error - # - increase number of instances - # - show that making one more now works without error - - -# Some test to illustrate what the formatting does -def test_error_str(file_regression): - res = create_error(1.0) - - # Don't worry about the value of instance_index - res_check = re.sub(r"instance_index=\d*", "instance_index=n", str(res)) - file_regression.check(res_check) - - -def test_error_pprint(file_regression): - res = create_error(1.0) - - # Don't worry about the value of instance_index - res_check = re.sub(r"instance_index=\d*", "instance_index=n", pretty(res)) - file_regression.check(res_check) - - -def test_error_html(file_regression): - res = create_error(1.0) +def test_create_multiple_errors(): + res = create_errors(np.arange(6)) - # Don't worry about the value of instance_index - res_check = re.sub(r"instance_index=\d*", "instance_index=n", res._repr_html_()) - file_regression.check(res_check, extension=".html") + for i, v in enumerate(res): + if i % 2 == 0: + assert res.code == 1 + assert res.message == "Even number supplied" + else: + assert res.code == 0 + assert res.message == "" From e5e8611d9a544eaf50454861a5a268b579472578 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 29 Aug 2025 13:41:38 +0200 Subject: [PATCH 19/49] Add failing tests --- tests/unit/test_error_v_creation.py | 5 ---- tests/unit/test_error_v_passing.py | 37 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_error_v_passing.py diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py index 3303090..8d46937 100644 --- a/tests/unit/test_error_v_creation.py +++ b/tests/unit/test_error_v_creation.py @@ -7,11 +7,6 @@ from example_fgen_basic.error_v import ErrorV from example_fgen_basic.error_v.creation import create_error, create_errors -# Tests to write: -# - passing derived types to Fortran (new test module) -# - retrieving multiple derived type instances from Fortran -# (basically checking the manager's auto-resizing of the number of instances) - def test_create_error_odd(): res = create_error(1.0) diff --git a/tests/unit/test_error_v_passing.py b/tests/unit/test_error_v_passing.py new file mode 100644 index 0000000..6311dfe --- /dev/null +++ b/tests/unit/test_error_v_passing.py @@ -0,0 +1,37 @@ +""" +Tests of `example_fgen_basic.error_v.passing` +""" + +import numpy as np + +from example_fgen_basic.error_v import ErrorV +from example_fgen_basic.error_v.passing import pass_error, pass_errors + + +def test_pass_error_odd(): + res = pass_error(ErrorV(code=1, message="hi")) + + assert res + + +def test_pass_error_even(): + res = pass_error(ErrorV(code=0)) + + assert not res + + +def test_pass_error_lots_of_repeated_calls(): + # We should be able to just keep calling `pass_error` + # without hitting segfaults or other weirdness. + # This is basically testing that we're freeing the temporary + # Fortran derived types correctly + # (and sort of a speed test, this shouldn't be noticeably slow) + # hence we may move this test somewhere more generic at some point. + for _ in range(1e5): + pass_error(ErrorV(code=0)) + + +def test_pass_multiple_errors(): + res = pass_errors([ErrorV(code=0), ErrorV(code=0), ErrorV(code=1)]) + + np.testing.assert_all_equal(res, np.array([True, True, False])) From f2596115dd3f88d293290899507d1d03dc2d1390 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 29 Aug 2025 14:31:47 +0200 Subject: [PATCH 20/49] Get create_error working as intended --- IMPLEMENTATION-NOTES.md | 4 +- src/example_fgen_basic/error_v/creation.py | 83 +++++++++++- .../error_v/creation_wrapper.f90 | 43 +++--- src/example_fgen_basic/error_v/error_v.f90 | 10 +- src/example_fgen_basic/error_v/error_v.py | 127 ++++-------------- .../error_v/error_v_manager.f90 | 108 ++++++++------- .../error_v/error_v_wrapper.f90 | 60 +++------ tests/unit/test_error_v_creation.py | 2 +- 8 files changed, 198 insertions(+), 239 deletions(-) diff --git a/IMPLEMENTATION-NOTES.md b/IMPLEMENTATION-NOTES.md index e09dceb..e939a42 100644 --- a/IMPLEMENTATION-NOTES.md +++ b/IMPLEMENTATION-NOTES.md @@ -72,7 +72,7 @@ Whenever we need to return a derived type to Python, we: it can just be that multiple levels of recursion are needed if you have derived types that themselves have derived type attributes 1. we then call the manager [TODO I think this will end up being wrapper, we can tighten the language later] - module's `finalise_instance_index` function to free the (temporary) instance + module's `finalise_instance` function to free the (temporary) instance that was used by the manager - this instance is no longer needed because all the data has been transferred to Python 1. we end up with a Python instance that has the result @@ -95,7 +95,7 @@ Whenever we need to pass a derived type to Fortran, we: and use this to call the Fortran function/subroutine of interest 1. return the result from Fortran back to Python 1. call the manager [TODO I think this will end up being wrapper, we can tighten the language later] - module's `finalise_instance_index` function to free the (temporary) instance + module's `finalise_instance` function to free the (temporary) instance that was used to pass the instance in the first place - this instance is no longer needed because all the data has been transferred and used by Fortran 1. we end up with the result of the Fortran callable back in Python diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py index 23ab07d..a4a30dc 100644 --- a/src/example_fgen_basic/error_v/creation.py +++ b/src/example_fgen_basic/error_v/creation.py @@ -1,26 +1,95 @@ """ -Demonstration of how to return an error (i.e. derived type) +Wrappers of `m_error_v_creation` [TODO think about naming and x-referencing] + +At the moment, all written by hand. +We will auto-generate this in future. """ from __future__ import annotations -from example_fgen_basic.error_v.error_v import ErrorV +from typing import TYPE_CHECKING + +from example_fgen_basic.error_v import ErrorV from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError +try: + from example_fgen_basic._lib import ( # type: ignore + m_error_v_w, + ) +except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover + raise CompiledExtensionNotFoundError("example_fgen_basic._lib.m_error_v_w") from exc try: from example_fgen_basic._lib import ( # type: ignore m_error_v_creation_w, ) except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover - raise CompiledExtensionNotFoundError("example._lib.m_error_v_creation_w") from exc + raise CompiledExtensionNotFoundError( + "example_fgen_basic._lib.m_error_v_creation_w" + ) from exc + +if TYPE_CHECKING: + # TODO: bring back in numpy type hints + NP_ARRAY_OF_INT = None def create_error(inv: int) -> ErrorV: """ - Create an instance of error (a wrapper around our Fortran derived type) + Create an error + + Parameters + ---------- + inv + Input value + + If odd, the error code is + [NO_ERROR_CODE][example_fgen_basic.error_v.NO_ERROR_CODE]. + If even, the error code is 1. + If a negative number is supplied, the error code is 2. + + Returns + ------- + : + Created error """ - instance_index = m_error_v_creation_w.create_error(inv) + # Get the result, but receiving an instance index rather than the object itself + instance_index: int = m_error_v_creation_w.create_error(inv) + + # Initialise the result from the received index + res = ErrorV.from_instance_index(instance_index) + + # Tell Fortran to finalise the object on the Fortran side + # (all data has been copied to Python now) + m_error_v_w.finalise_instance(instance_index) + + return res + + +def create_errors(invs: NP_ARRAY_OF_INT) -> tuple[ErrorV, ...]: + """ + Create a number of errors + + Parameters + ---------- + invs + Input values from which to create errors + + For each value in `invs`, + if the value is even, an error is created, + if the value is odd, an error with a no error code is created. + + Returns + ------- + : + Created errors + """ + # Get the result, but receiving an instance index rather than the object itself + instance_indexes: list[int] = m_error_v_creation_w.create_errors(invs) + + # Initialise the result from the received index + res = [ErrorV.from_instance_index(i) for i in instance_indexes] - error = ErrorV(instance_index) + # Tell Fortran to finalise the object on the Fortran side + # (all data has been copied to Python now) + m_error_v_w.finalise_instances(instance_indexes) - return error + return res diff --git a/src/example_fgen_basic/error_v/creation_wrapper.f90 b/src/example_fgen_basic/error_v/creation_wrapper.f90 index a105a92..b45584c 100644 --- a/src/example_fgen_basic/error_v/creation_wrapper.f90 +++ b/src/example_fgen_basic/error_v/creation_wrapper.f90 @@ -4,17 +4,15 @@ !> Generation to be automated in future (including docstrings of some sort). module m_error_v_creation_w - use iso_c_binding, only: c_ptr, c_loc - ! => allows us to rename on import to avoid clashes + ! "o_" for original (TODO: better naming convention) use m_error_v_creation, only: o_create_error => create_error use m_error_v, only: ErrorV ! The manager module, which makes this all work use m_error_v_manager, only: & - error_v_manager_get_free_instance_number => get_free_instance_number, & - error_v_manager_associate_pointer_with_instance => associate_pointer_with_instance - ! TODO: finalisation + error_v_manager_get_available_instance_index => get_available_instance_index, & + error_v_manager_set_instance_index_to => set_instance_index_to implicit none private @@ -23,38 +21,33 @@ module m_error_v_creation_w contains - subroutine create_error(inv, res_instance_index) - ! Needs to be subroutine to have the created instance persist I think - ! (we can check) - ! function create_error(inv) result(res_instance_index) + function create_error(inv) result(res_instance_index) + !! Wrapper around `m_error_v_creation.create_error` (TODO: x-ref) integer, intent(in) :: inv !! Input value to use to create the error + !! + !! See docstring of `m_error_v_creation.create_error` for details. + !! [TODO: x-ref] - integer, intent(out) :: res_instance_index + integer :: res_instance_index !! Instance index of the result ! ! This is the major trick for wrapping. ! We return instance indexes (integers) to Python rather than the instance itself. - type(ErrorV), pointer :: res - - ! This is the other trick for wrapping. - ! We have to ensure that we have correctly associated pointers - ! with the derived type instances we want to 'pass' across the Python-Fortran interface. - ! Once we've done this, we can then set them more or less like normal derived types. - res_instance_index = error_v_manager_get_free_instance_number() - ! We have to associate res with the right index - ! before we can set it to the output of the function call - ! (in this case `o_create_error`). - call error_v_manager_associate_pointer_with_instance(res_instance_index, res) + type(ErrorV) :: res - ! Use the pointer more or less like a normal instance of the derived type + ! Do the Fortran call res = o_create_error(inv) - ! Ensure that the instance index is set correctly - res % instance_index = res_instance_index + ! Get the instance index to return to Python + call error_v_manager_get_available_instance_index(res_instance_index) + + ! Set the derived type value in the manager's array, + ! ready for its attributes to be retrieved from Python. + call error_v_manager_set_instance_index_to(res_instance_index, res) - end subroutine create_error + end function create_error end module m_error_v_creation_w diff --git a/src/example_fgen_basic/error_v/error_v.f90 b/src/example_fgen_basic/error_v/error_v.f90 index ae47aaa..a05363f 100644 --- a/src/example_fgen_basic/error_v/error_v.f90 +++ b/src/example_fgen_basic/error_v/error_v.f90 @@ -8,25 +8,20 @@ !> with the convention that a code of 0 indicates no error. module m_error_v - ! TODO: switch `m_` prefix to project name prefix - ! (e.g. fpyfgen, "Fortran-code for Python-Fortran wrapper Generator") - ! (same idea as `tomlf_` prefix) - ! TODO: move this to standalone repo (eventually) - use fpyfgen_base_finalisable, only: BaseFinalisable - implicit none private integer, parameter, public :: NO_ERROR_CODE = 0 !! Code that indicates no error - type, extends(BaseFinalisable), public :: ErrorV + type, public :: ErrorV !! Error value integer :: code = 1 !! Error code character(len=128) :: message = "" + !! Error message ! TODO: think about making the message allocatable to handle long messages ! TODO: think about adding idea of critical @@ -47,7 +42,6 @@ module m_error_v interface ErrorV !! Constructor interface - see build (TODO: figure out cross-ref syntax) for details - ! This is what allows us to do ErrorV(...) when using this class module procedure :: constructor end interface ErrorV diff --git a/src/example_fgen_basic/error_v/error_v.py b/src/example_fgen_basic/error_v/error_v.py index 1218606..7fc12c8 100644 --- a/src/example_fgen_basic/error_v/error_v.py +++ b/src/example_fgen_basic/error_v/error_v.py @@ -1,18 +1,14 @@ """ -Wrapper of the Fortran :class:`ErrorV` +Python equivalent of the Fortran `ErrorV` class [TODO: x-refs] + +At the moment, all written by hand. +We will auto-generate this in future. """ from __future__ import annotations -from typing import Any - from attrs import define -from example_fgen_basic.pyfgen_runtime.base_finalisable import ( - FinalisableWrapperBase, - check_initialised, - execute_finalise_on_fail, -) from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError try: @@ -20,114 +16,47 @@ m_error_v_w, ) except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover - raise CompiledExtensionNotFoundError("example._lib.m_error_v_w") from exc + raise CompiledExtensionNotFoundError("example_fgen_basic._lib.m_error_v_w") from exc + +NO_ERROR_CODE = 0 +"""Code that indicates no error""" @define -class ErrorV(FinalisableWrapperBase): +class ErrorV: """ - TODO: auto docstring e.g. "Wrapper around the Fortran :class:`ErrorV`" + Error value """ - # Bug in Ipython pretty hence have to put this on every object? - def _repr_pretty_(self, p: Any, cycle: bool) -> None: - """ - Get pretty representation of self - - Used by IPython notebooks and other tools - """ - super()._repr_pretty_(p=p, cycle=cycle) + code: int = 1 + """Error code""" - @property - def exposed_attributes(self) -> tuple[str, ...]: - """ - Attributes exposed by this wrapper - """ - return ("code", "message") + message: str = "" + """Error message""" - # TODO: context manager @classmethod - def from_new_connection(cls) -> ErrorV: + def from_instance_index(cls, instance_index: int) -> ErrorV: """ - Initialise from a new connection + Initialise from an instance index received from Fortran - The user is responsible for releasing this connection - using :attr:`~finalise` when it is no longer needed. - Alternatively an [ErrorVContext][] - can be used to handle the finalisation using a context manager. + Parameters + ---------- + instance_index + Instance index received from Fortran Returns ------- : - A new instance with a unique instance index - - Raises - ------ - WrapperErrorUnknownCause - If a new instance could not be allocated - - This could occur if too many instances are allocated at any one time + Initialised index """ - instance_ptr: int = m_error_v_w.get_free_instance_number() + # Different wrapping strategies are needed - return cls(instance_ptr) + # Integer is very simple + code = m_error_v_w.get_code(instance_index) - @classmethod - def from_build_args( - cls, - code: int, - message: str = "", - ) -> ErrorV: - """ - Build the class (including connecting to Fortran) - """ - out = cls.from_new_connection() - # TODO: remove or update this construct when we have result types - execute_finalise_on_fail( - out, - m_error_v_w.instance_build, - code=code, - message=message, - ) - - return out - - @check_initialised - def finalise(self) -> None: - """ - Close the connection with the Fortran module - """ - m_error_v_w.instance_finalise(self.instance_index) - self._uninitialise_instance_index() + # String requires decode + message = m_error_v_w.get_message(instance_index).decode() - @property - @check_initialised - def code(self) -> int: - """ - Error code - - Returns - ------- - : - Error code, retrieved from Fortran - """ - code: int = m_error_v_w.iget_code(instance_index=self.instance_index) - - return code - - @property - @check_initialised - def message(self) -> str: - """ - Error message - - Returns - ------- - : - Error message, retrieved from Fortran - """ - message: str = m_error_v_w.iget_message( - instance_index=self.instance_index - ).decode() + res = cls(code=code, message=message) - return message + return res diff --git a/src/example_fgen_basic/error_v/error_v_manager.f90 b/src/example_fgen_basic/error_v/error_v_manager.f90 index 61e01a7..5f92f3d 100644 --- a/src/example_fgen_basic/error_v/error_v_manager.f90 +++ b/src/example_fgen_basic/error_v/error_v_manager.f90 @@ -6,76 +6,87 @@ ! TODO: make it possible to reallocate the number of instances module m_error_v_manager - use fpyfgen_derived_type_manager_helpers, only: finalise_derived_type_instance_number, & - get_derived_type_free_instance_number use m_error_v, only: ErrorV implicit none private - integer, public, parameter :: N_INSTANCES_DEFAULT = 4096 - !! Default maximum number of instances which can be created simultaneously - ! - ! TODO: allow reallocation if possible + type(ErrorV), dimension(1) :: instance_array + logical, dimension(1) :: instance_available = .true. - ! This is the other trick, we hold an array of instances - ! for tracking what is being passed back and forth across the interface. - type(ErrorV), target, dimension(N_INSTANCES_DEFAULT) :: instance_array - logical, dimension(N_INSTANCES_DEFAULT) :: instance_available = .true. - - public :: get_free_instance_number, & - associate_pointer_with_instance, & - finalise_instance + public :: finalise_instance, get_available_instance_index, get_instance, set_instance_index_to contains - function get_free_instance_number() result(instance_index) - !! Get the index of a free instance + subroutine finalise_instance(instance_index) + !! Finalise an instance - integer :: instance_index - !! Free instance index + integer, intent(in) :: instance_index + !! Index of the instance to finalise - call get_derived_type_free_instance_number( & - instance_index, & - N_INSTANCES_DEFAULT, & - instance_available, & - instance_array & - ) + call check_index_claimed(instance_index) - end function get_free_instance_number + call instance_array(instance_index) % finalise() + instance_available(instance_index) = .true. - ! Might be a better way to do this as the pointers are a bit confusing, let's see - subroutine associate_pointer_with_instance(instance_index, instance_pointer) - !! Associate a pointer with the instance corresponding to the given model index - !! - !! Stops execution if the instance has not already been initialised. + end subroutine finalise_instance + + subroutine get_available_instance_index(available_instance_index) + !! Get a free instance index + + ! TODO: think through whether race conditions are possible + ! e.g. while returning a free index number to one Python call + ! a different one can be looking up a free instance index at the same time + ! and something goes wrong (maybe we need a lock) + + integer, intent(out) :: available_instance_index + !! Available instance index + + integer :: i + + do i = 1, size(instance_array) + + if (instance_available(i)) then + + instance_available(i) = .false. + available_instance_index = i + return + + end if + + end do + + ! TODO: switch to returning a Result type with an error set + print *, "No free indexes" + error stop 1 + + end subroutine get_available_instance_index + + ! Change to pure function when we update check_index_claimed to be pure + function get_instance(instance_index) result(inst) integer, intent(in) :: instance_index - !! Index of the instance to point to + !! Index in `instance_array` of which to set the value equal to `val` - type(ErrorV), pointer, intent(inout) :: instance_pointer - !! Pointer to associate + type(ErrorV) :: inst + !! Instance at `instance_array(instance_index)` call check_index_claimed(instance_index) - instance_pointer => instance_array(instance_index) + inst = instance_array(instance_index) - end subroutine associate_pointer_with_instance + end function get_instance - subroutine finalise_instance(instance_index) - !! Finalise an instance + subroutine set_instance_index_to(instance_index, val) integer, intent(in) :: instance_index - !! Index of the instance to finalise + !! Index in `instance_array` of which to set the value equal to `val` + + type(ErrorV), intent(in) :: val call check_index_claimed(instance_index) - call finalise_derived_type_instance_number( & - instance_index, & - N_INSTANCES_DEFAULT, & - instance_available, & - instance_array & - ) + instance_array(instance_index) = val - end subroutine finalise_instance + end subroutine set_instance_index_to subroutine check_index_claimed(instance_index) !! Check that an index has already been claimed @@ -97,13 +108,6 @@ subroutine check_index_claimed(instance_index) error stop 1 end if - if (instance_array(instance_index) % instance_index < 1) then - ! TODO: switch to errors here - will require some thinking - print *, "Index ", instance_index, " is associated with an instance that has instance index < 1", & - "instance's instance_index attribute ", instance_array(instance_index) % instance_index - error stop 1 - end if - end subroutine check_index_claimed end module m_error_v_manager diff --git a/src/example_fgen_basic/error_v/error_v_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_wrapper.f90 index dacc5d2..4157472 100644 --- a/src/example_fgen_basic/error_v/error_v_wrapper.f90 +++ b/src/example_fgen_basic/error_v/error_v_wrapper.f90 @@ -9,49 +9,19 @@ module m_error_v_w ! The manager module, which makes this all work use m_error_v_manager, only: & - error_v_manager_get_free_instance_number => get_free_instance_number, & error_v_manager_finalise_instance => finalise_instance, & - error_v_manager_associate_pointer_with_instance => associate_pointer_with_instance - ! TODO: build and finalisation + error_v_manager_get_instance => get_instance implicit none private - public :: get_free_instance_number, instance_build, instance_finalise, & - iget_code, iget_message + public :: finalise_instance, & + get_code, get_message contains - function get_free_instance_number() result(instance_index) - - integer :: instance_index - - instance_index = error_v_manager_get_free_instance_number() - - end function get_free_instance_number - - subroutine instance_build(instance_index, code, message) - !> Build an instance - - integer, intent(in) :: instance_index - !! Instance index - ! - ! This is the major trick for wrapping. - ! We pass instance indexes (integers) to Python rather than the instance itself. - - integer, intent(in) :: code - character(len=*), optional, intent(in) :: message - - type(ErrorV), pointer :: instance - - call error_v_manager_associate_pointer_with_instance(instance_index, instance) - - call instance % build(code, message) - - end subroutine instance_build - - subroutine instance_finalise(instance_index) - !> Finalise an instance + subroutine finalise_instance(instance_index) + !! Finalise an instance integer, intent(in) :: instance_index !! Instance index @@ -61,13 +31,13 @@ subroutine instance_finalise(instance_index) call error_v_manager_finalise_instance(instance_index) - end subroutine instance_finalise + end subroutine finalise_instance - ! Full set of wrapping strategies to pass different types in e.g. + ! Full set of wrapping strategies to get/pass different types in e.g. ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 ! (we will do a full re-write of the code which generates this, ! but the strategies will probably stay as they are) - subroutine iget_code( & + subroutine get_code( & instance_index, & code & ) @@ -76,15 +46,15 @@ subroutine iget_code( & integer, intent(out) :: code - type(ErrorV), pointer :: instance + type(ErrorV) :: instance - call error_v_manager_associate_pointer_with_instance(instance_index, instance) + instance = error_v_manager_get_instance(instance_index) code = instance % code - end subroutine iget_code + end subroutine get_code - subroutine iget_message( & + subroutine get_message( & instance_index, & message & ) @@ -94,12 +64,12 @@ subroutine iget_message( & ! TODO: make this variable length character(len=128), intent(out) :: message - type(ErrorV), pointer :: instance + type(ErrorV) :: instance - call error_v_manager_associate_pointer_with_instance(instance_index, instance) + instance = error_v_manager_get_instance(instance_index) message = instance % message - end subroutine iget_message + end subroutine get_message end module m_error_v_w diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py index 8d46937..e5e8b7e 100644 --- a/tests/unit/test_error_v_creation.py +++ b/tests/unit/test_error_v_creation.py @@ -43,7 +43,7 @@ def test_create_error_lots_of_repeated_calls(): # Fortran derived types correctly # (and sort of a speed test, this shouldn't be noticeably slow) # hence we may move this test somewhere more generic at some point. - for _ in range(1e5): + for _ in range(int(1e5)): create_error(1) From 5fceeef8ce91c21d0adce3743567454636d8bc98 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 29 Aug 2025 14:49:04 +0200 Subject: [PATCH 21/49] Something that compiles --- src/example_fgen_basic/error_v/creation.f90 | 28 +++++++++- .../error_v/creation_wrapper.f90 | 55 ++++++++++++++++++- .../error_v/error_v_manager.f90 | 38 ++++++++++++- 3 files changed, 114 insertions(+), 7 deletions(-) diff --git a/src/example_fgen_basic/error_v/creation.f90 b/src/example_fgen_basic/error_v/creation.f90 index 85d43e7..50935ed 100644 --- a/src/example_fgen_basic/error_v/creation.f90 +++ b/src/example_fgen_basic/error_v/creation.f90 @@ -11,7 +11,7 @@ module m_error_v_creation implicit none private - public :: create_error + public :: create_error, create_errors contains @@ -41,4 +41,30 @@ function create_error(inv) result(err) end function create_error + function create_errors(invs, n) result(errs) + !! Create a number of errors + !! + !! If an odd number is supplied, the error code is no error (TODO: cross-ref). + !! If an even number is supplied, the error code is 1. + !! If a negative number is supplied, the error code is 2. + + integer, dimension(n), intent(in) :: invs + !! Values to use to create the error + + integer, intent(in) :: n + !! Number of values to create + + type(ErrorV), dimension(n) :: errs + !! Created errors + + integer :: i + + do i = 1, n + + errs(i) = create_error(invs(i)) + + end do + + end function create_errors + end module m_error_v_creation diff --git a/src/example_fgen_basic/error_v/creation_wrapper.f90 b/src/example_fgen_basic/error_v/creation_wrapper.f90 index b45584c..34b3619 100644 --- a/src/example_fgen_basic/error_v/creation_wrapper.f90 +++ b/src/example_fgen_basic/error_v/creation_wrapper.f90 @@ -6,18 +6,21 @@ module m_error_v_creation_w ! => allows us to rename on import to avoid clashes ! "o_" for original (TODO: better naming convention) - use m_error_v_creation, only: o_create_error => create_error + use m_error_v_creation, only: & + o_create_error => create_error, & + o_create_errors => create_errors use m_error_v, only: ErrorV ! The manager module, which makes this all work use m_error_v_manager, only: & error_v_manager_get_available_instance_index => get_available_instance_index, & - error_v_manager_set_instance_index_to => set_instance_index_to + error_v_manager_set_instance_index_to => set_instance_index_to, & + error_v_manager_ensure_instance_array_size_is_at_least => ensure_instance_array_size_is_at_least implicit none private - public :: create_error + public :: create_error, create_errors contains @@ -41,6 +44,8 @@ function create_error(inv) result(res_instance_index) ! Do the Fortran call res = o_create_error(inv) + call error_v_manager_ensure_instance_array_size_is_at_least(1) + ! Get the instance index to return to Python call error_v_manager_get_available_instance_index(res_instance_index) @@ -50,4 +55,48 @@ function create_error(inv) result(res_instance_index) end function create_error + function create_errors(invs, n) result(res_instance_indexes) + !! Wrapper around `m_error_v_creation.create_errors` (TODO: x-ref) + + integer, dimension(n), intent(in) :: invs + !! Input value to use to create the error + !! + !! See docstring of `m_error_v_creation.create_error` for details. + !! [TODO: x-ref] + + integer, intent(in) :: n + !! Number of values to create + + integer, dimension(n) :: res_instance_indexes + !! Instance indexes of the result + ! + ! This is the major trick for wrapping. + ! We return instance indexes (integers) to Python rather than the instance itself. + + type(ErrorV), dimension(n) :: res + + integer :: i, tmp + + ! Lots of ways resizing could work. + ! Optimising could be very tricky. + ! Just do something stupid for now to see the pattern. + call error_v_manager_ensure_instance_array_size_is_at_least(n) + + ! Do the Fortran call + res = o_create_errors(invs, n) + + do i = 1, n + + ! Get the instance index to return to Python + call error_v_manager_get_available_instance_index(tmp) + ! Set the derived type value in the manager's array, + ! ready for its attributes to be retrieved from Python. + call error_v_manager_set_instance_index_to(tmp, res(i)) + ! Set the result in the output array + res_instance_indexes(i) = tmp + + end do + + end function create_errors + end module m_error_v_creation_w diff --git a/src/example_fgen_basic/error_v/error_v_manager.f90 b/src/example_fgen_basic/error_v/error_v_manager.f90 index 5f92f3d..1220e4a 100644 --- a/src/example_fgen_basic/error_v/error_v_manager.f90 +++ b/src/example_fgen_basic/error_v/error_v_manager.f90 @@ -11,10 +11,12 @@ module m_error_v_manager implicit none private - type(ErrorV), dimension(1) :: instance_array - logical, dimension(1) :: instance_available = .true. + type(ErrorV), dimension(:), allocatable :: instance_array + logical, dimension(:), allocatable :: instance_available - public :: finalise_instance, get_available_instance_index, get_instance, set_instance_index_to + ! TODO: think about ordering here, alphabetical probably easiest + public :: finalise_instance, get_available_instance_index, get_instance, set_instance_index_to, & + ensure_instance_array_size_is_at_least contains @@ -110,4 +112,34 @@ subroutine check_index_claimed(instance_index) end subroutine check_index_claimed + subroutine ensure_instance_array_size_is_at_least(n) + !! Ensure that `instance_array` and `instance_available` have at least `n` slots + + integer, intent(in) :: n + + type(ErrorV), dimension(:), allocatable :: tmp_instances + logical, dimension(:), allocatable :: tmp_available + + if (.not. allocated(instance_array)) then + + allocate(instance_array(n)) + + allocate(instance_available(n)) + ! Race conditions ? + instance_available = .true. + + elseif (size(instance_available) < n) then + + allocate(tmp_instances(n)) + tmp_instances(1:size(instance_array)) = instance_array + call move_alloc(tmp_instances, instance_array) + + allocate(tmp_available(n)) + tmp_available(1:size(instance_available)) = instance_available + call move_alloc(tmp_available, instance_available) + + end if + + end subroutine ensure_instance_array_size_is_at_least + end module m_error_v_manager From 274a61cc636c1779b0263ed9095331066c0f1dec Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 29 Aug 2025 14:53:30 +0200 Subject: [PATCH 22/49] Pass multiple return tests --- src/example_fgen_basic/error_v/creation.py | 4 ++-- .../error_v/error_v_manager.f90 | 3 +++ .../error_v/error_v_wrapper.f90 | 20 ++++++++++++++++++- tests/unit/test_error_v_creation.py | 8 ++++---- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py index a4a30dc..3e18787 100644 --- a/src/example_fgen_basic/error_v/creation.py +++ b/src/example_fgen_basic/error_v/creation.py @@ -83,10 +83,10 @@ def create_errors(invs: NP_ARRAY_OF_INT) -> tuple[ErrorV, ...]: Created errors """ # Get the result, but receiving an instance index rather than the object itself - instance_indexes: list[int] = m_error_v_creation_w.create_errors(invs) + instance_indexes: NP_ARRAY_OF_INT = m_error_v_creation_w.create_errors(invs) # Initialise the result from the received index - res = [ErrorV.from_instance_index(i) for i in instance_indexes] + res = tuple(ErrorV.from_instance_index(i) for i in instance_indexes) # Tell Fortran to finalise the object on the Fortran side # (all data has been copied to Python now) diff --git a/src/example_fgen_basic/error_v/error_v_manager.f90 b/src/example_fgen_basic/error_v/error_v_manager.f90 index 1220e4a..dd3ea11 100644 --- a/src/example_fgen_basic/error_v/error_v_manager.f90 +++ b/src/example_fgen_basic/error_v/error_v_manager.f90 @@ -112,6 +112,9 @@ subroutine check_index_claimed(instance_index) end subroutine check_index_claimed + ! TODO: think about exposing this to Python + ! so power users can ensure there are enough arrays at the start + ! rather than relying on automatic resizing. subroutine ensure_instance_array_size_is_at_least(n) !! Ensure that `instance_array` and `instance_available` have at least `n` slots diff --git a/src/example_fgen_basic/error_v/error_v_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_wrapper.f90 index 4157472..0b70b92 100644 --- a/src/example_fgen_basic/error_v/error_v_wrapper.f90 +++ b/src/example_fgen_basic/error_v/error_v_wrapper.f90 @@ -15,7 +15,7 @@ module m_error_v_w implicit none private - public :: finalise_instance, & + public :: finalise_instance, finalise_instances, & get_code, get_message contains @@ -33,6 +33,24 @@ subroutine finalise_instance(instance_index) end subroutine finalise_instance + + subroutine finalise_instances(instance_indexes) + !! Finalise an instance + + integer, dimension(:), intent(in) :: instance_indexes + !! Instance indexes to finalise + ! + ! This is the major trick for wrapping. + ! We pass instance indexes (integers) to Python rather than the instance itself. + + integer :: i + + do i = 1, size(instance_indexes) + call error_v_manager_finalise_instance(instance_indexes(i)) + end do + + end subroutine finalise_instances + ! Full set of wrapping strategies to get/pass different types in e.g. ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 ! (we will do a full re-write of the code which generates this, diff --git a/tests/unit/test_error_v_creation.py b/tests/unit/test_error_v_creation.py index e5e8b7e..b3d3c7e 100644 --- a/tests/unit/test_error_v_creation.py +++ b/tests/unit/test_error_v_creation.py @@ -52,8 +52,8 @@ def test_create_multiple_errors(): for i, v in enumerate(res): if i % 2 == 0: - assert res.code == 1 - assert res.message == "Even number supplied" + assert v.code == 1 + assert v.message == "Even number supplied" else: - assert res.code == 0 - assert res.message == "" + assert v.code == 0 + assert v.message == "" From 52a6ec86c5343e2a9c75dc423aae50a50e8f1002 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 29 Aug 2025 15:36:29 +0200 Subject: [PATCH 23/49] Pass all tests --- meson.build | 5 +- .../error_v/error_v_manager.f90 | 25 +- .../error_v/error_v_wrapper.f90 | 38 ++- src/example_fgen_basic/error_v/passing.f90 | 57 +++++ src/example_fgen_basic/error_v/passing.py | 96 +++++++ .../error_v/passing_wrapper.f90 | 81 ++++++ .../pyfgen_runtime/base_finalisable.py | 218 ---------------- .../pyfgen_runtime/formatting.py | 234 ------------------ tests/unit/test_error_v_creation.f90 | 4 +- tests/unit/test_error_v_passing.py | 4 +- 10 files changed, 296 insertions(+), 466 deletions(-) create mode 100644 src/example_fgen_basic/error_v/passing.f90 create mode 100644 src/example_fgen_basic/error_v/passing.py create mode 100644 src/example_fgen_basic/error_v/passing_wrapper.f90 delete mode 100644 src/example_fgen_basic/pyfgen_runtime/base_finalisable.py delete mode 100644 src/example_fgen_basic/pyfgen_runtime/formatting.py diff --git a/meson.build b/meson.build index 276eef9..d9e0e4b 100644 --- a/meson.build +++ b/meson.build @@ -53,6 +53,7 @@ if pyprojectwheelbuild_enabled srcs = files( 'src/example_fgen_basic/error_v/creation_wrapper.f90', 'src/example_fgen_basic/error_v/error_v_wrapper.f90', + 'src/example_fgen_basic/error_v/passing_wrapper.f90', 'src/example_fgen_basic/get_wavelength_wrapper.f90', ) @@ -62,6 +63,7 @@ if pyprojectwheelbuild_enabled 'src/example_fgen_basic/error_v/creation.f90', 'src/example_fgen_basic/error_v/error_v.f90', 'src/example_fgen_basic/error_v/error_v_manager.f90', + 'src/example_fgen_basic/error_v/passing.f90', 'src/example_fgen_basic/fpyfgen/base_finalisable.f90', 'src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90', 'src/example_fgen_basic/get_wavelength.f90', @@ -75,12 +77,11 @@ if pyprojectwheelbuild_enabled 'src/example_fgen_basic/error_v/__init__.py', 'src/example_fgen_basic/error_v/creation.py', 'src/example_fgen_basic/error_v/error_v.py', + 'src/example_fgen_basic/error_v/passing.py', 'src/example_fgen_basic/exceptions.py', 'src/example_fgen_basic/get_wavelength.py', 'src/example_fgen_basic/pyfgen_runtime/__init__.py', - 'src/example_fgen_basic/pyfgen_runtime/base_finalisable.py', 'src/example_fgen_basic/pyfgen_runtime/exceptions.py', - 'src/example_fgen_basic/pyfgen_runtime/formatting.py', ) # The ancillary library, diff --git a/src/example_fgen_basic/error_v/error_v_manager.f90 b/src/example_fgen_basic/error_v/error_v_manager.f90 index dd3ea11..34bfa53 100644 --- a/src/example_fgen_basic/error_v/error_v_manager.f90 +++ b/src/example_fgen_basic/error_v/error_v_manager.f90 @@ -15,11 +15,29 @@ module m_error_v_manager logical, dimension(:), allocatable :: instance_available ! TODO: think about ordering here, alphabetical probably easiest - public :: finalise_instance, get_available_instance_index, get_instance, set_instance_index_to, & + public :: build_instance, finalise_instance, get_available_instance_index, get_instance, set_instance_index_to, & ensure_instance_array_size_is_at_least contains + function build_instance(code, message) result(instance_index) + !! Build an instance + + integer, intent(in) :: code + !! Error code + + character(len=*), optional, intent(in) :: message + !! Error message + + integer :: instance_index + !! Index of the built instance + + call ensure_instance_array_size_is_at_least(1) + call get_available_instance_index(instance_index) + call instance_array(instance_index) % build(code=code, message=message) + + end function build_instance + subroutine finalise_instance(instance_index) !! Finalise an instance @@ -59,7 +77,6 @@ subroutine get_available_instance_index(available_instance_index) end do ! TODO: switch to returning a Result type with an error set - print *, "No free indexes" error stop 1 end subroutine get_available_instance_index @@ -112,9 +129,6 @@ subroutine check_index_claimed(instance_index) end subroutine check_index_claimed - ! TODO: think about exposing this to Python - ! so power users can ensure there are enough arrays at the start - ! rather than relying on automatic resizing. subroutine ensure_instance_array_size_is_at_least(n) !! Ensure that `instance_array` and `instance_available` have at least `n` slots @@ -139,6 +153,7 @@ subroutine ensure_instance_array_size_is_at_least(n) allocate(tmp_available(n)) tmp_available(1:size(instance_available)) = instance_available + tmp_available(size(instance_available) + 1:size(tmp_available)) = .true. call move_alloc(tmp_available, instance_available) end if diff --git a/src/example_fgen_basic/error_v/error_v_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_wrapper.f90 index 0b70b92..7cde3a0 100644 --- a/src/example_fgen_basic/error_v/error_v_wrapper.f90 +++ b/src/example_fgen_basic/error_v/error_v_wrapper.f90 @@ -9,17 +9,41 @@ module m_error_v_w ! The manager module, which makes this all work use m_error_v_manager, only: & + error_v_manager_build_instance => build_instance, & error_v_manager_finalise_instance => finalise_instance, & - error_v_manager_get_instance => get_instance + error_v_manager_get_instance => get_instance, & + error_v_manager_ensure_instance_array_size_is_at_least => ensure_instance_array_size_is_at_least implicit none private - public :: finalise_instance, finalise_instances, & + public :: build_instance, finalise_instance, finalise_instances, & + ensure_at_least_n_instances_can_be_passed_simultaneously, & get_code, get_message contains + subroutine build_instance(code, message, instance_index) + !! Build an instance + + integer, intent(in) :: code + !! Error code + !! + !! Use [TODO: figure out xref] `NO_ERROR_CODE` if there is no error + + character(len=*), optional, intent(in) :: message + !! Error message + + integer, intent(out) :: instance_index + !! Instance index of the built instance + ! + ! This is the major trick for wrapping. + ! We pass instance indexes (integers) to Python rather than the instance itself. + + instance_index = error_v_manager_build_instance(code, message) + + end subroutine build_instance + subroutine finalise_instance(instance_index) !! Finalise an instance @@ -33,7 +57,6 @@ subroutine finalise_instance(instance_index) end subroutine finalise_instance - subroutine finalise_instances(instance_indexes) !! Finalise an instance @@ -51,6 +74,15 @@ subroutine finalise_instances(instance_indexes) end subroutine finalise_instances + subroutine ensure_at_least_n_instances_can_be_passed_simultaneously(n) + !! Ensure that at least `n` instances of `ErrorV` can be passed via the manager simultaneously + + integer, intent(in) :: n + + call error_v_manager_ensure_instance_array_size_is_at_least(n) + + end subroutine ensure_at_least_n_instances_can_be_passed_simultaneously + ! Full set of wrapping strategies to get/pass different types in e.g. ! https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/tests/test-data/exposed_attrs/src/exposed_attrs/exposed_attrs_wrapped.f90 ! (we will do a full re-write of the code which generates this, diff --git a/src/example_fgen_basic/error_v/passing.f90 b/src/example_fgen_basic/error_v/passing.f90 new file mode 100644 index 0000000..be0fc9a --- /dev/null +++ b/src/example_fgen_basic/error_v/passing.f90 @@ -0,0 +1,57 @@ +!> Error passing +!> +!> A very basic demo to get the idea. +! +! TODO: discuss - we should probably have some convention for module names. +! The hard part is avoiding them becoming too long... +module m_error_v_passing + + use m_error_v, only: ErrorV, NO_ERROR_CODE + + implicit none + private + + public :: pass_error, pass_errors + +contains + + function pass_error(inv) result(is_err) + !! Pass an error + !! + !! If an error is supplied, we return `.true.`, otherwise `.false.`. + + type(ErrorV), intent(in) :: inv + !! Input error value + + logical :: is_err + !! Whether `inv` is an error or not + + is_err = (inv % code /= NO_ERROR_CODE) + + end function pass_error + + function pass_errors(invs, n) result(is_errs) + !! Pass a number of errors + !! + !! For each value in `invs`, if an error is supplied, we return `.true.`, otherwise `.false.`. + + type(ErrorV), dimension(n), intent(in) :: invs + !! Input error values + + integer, intent(in) :: n + !! Number of values being passed + + logical, dimension(n) :: is_errs + !! Whether each value in `invs` is an error or not + + integer :: i + + do i = 1, n + + is_errs(i) = pass_error(invs(i)) + + end do + + end function pass_errors + +end module m_error_v_passing diff --git a/src/example_fgen_basic/error_v/passing.py b/src/example_fgen_basic/error_v/passing.py new file mode 100644 index 0000000..4f832ae --- /dev/null +++ b/src/example_fgen_basic/error_v/passing.py @@ -0,0 +1,96 @@ +""" +Wrappers of `m_error_v_passing` [TODO think about naming and x-referencing] + +At the moment, all written by hand. +We will auto-generate this in future. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import numpy as np + +from example_fgen_basic.error_v import ErrorV +from example_fgen_basic.pyfgen_runtime.exceptions import CompiledExtensionNotFoundError + +try: + from example_fgen_basic._lib import ( # type: ignore + m_error_v_w, + ) +except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover + raise CompiledExtensionNotFoundError("example_fgen_basic._lib.m_error_v_w") from exc +try: + from example_fgen_basic._lib import ( # type: ignore + m_error_v_passing_w, + ) +except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover + raise CompiledExtensionNotFoundError( + "example_fgen_basic._lib.m_error_v_passing_w" + ) from exc + +if TYPE_CHECKING: + # TODO: bring back in numpy type hints + NP_ARRAY_OF_INT = None + NP_ARRAY_OF_BOOL = None + + +def pass_error(inv: ErrorV) -> bool: + """ + Pass an error to Fortran + + Parameters + ---------- + inv + Input value to pass to Fortran + + Returns + ------- + : + If `inv` is an error, `True`, otherwise `False`. + """ + # Tell Fortran to build the object on the Fortran side + instance_index: int = m_error_v_w.build_instance(code=inv.code, message=inv.message) + + # Call the Fortran function + # Boolean wrapping strategy, have to cast to bool + res_raw: int = m_error_v_passing_w.pass_error(instance_index) + res = bool(res_raw) + + # Tell Fortran to finalise the object on the Fortran side + # (all data has been used for the call now) + m_error_v_w.finalise_instance(instance_index) + + return res + + +def pass_errors(invs: tuple[ErrorV, ...]) -> NP_ARRAY_OF_BOOL: + """ + Pass a number of errors to Fortran + + Parameters + ---------- + invs + Errors to pass to Fortran + + Returns + ------- + : + Whether each value in `invs` is an error or not + """ + # Controlling memory from the Python side + m_error_v_w.ensure_at_least_n_instances_can_be_passed_simultaneously(len(invs)) + # TODO: consider adding `build_instances` too, might be headache + instance_indexes: NP_ARRAY_OF_INT = np.array( + [m_error_v_w.build_instance(code=inv.code, message=inv.message) for inv in invs] + ) + + # Convert the result to boolean + res_raw = m_error_v_passing_w.pass_errors(instance_indexes, n=instance_indexes.size) + res = res_raw.astype(bool) + + # Tell Fortran to finalise the objects on the Fortran side + # (all data has been copied to Python now) + m_error_v_w.finalise_instances(instance_indexes) + + return res diff --git a/src/example_fgen_basic/error_v/passing_wrapper.f90 b/src/example_fgen_basic/error_v/passing_wrapper.f90 new file mode 100644 index 0000000..7c3c96a --- /dev/null +++ b/src/example_fgen_basic/error_v/passing_wrapper.f90 @@ -0,0 +1,81 @@ +!> Wrapper for interfacing `m_error_v_passing` with Python +!> +!> Written by hand here. +!> Generation to be automated in future (including docstrings of some sort). +module m_error_v_passing_w + + ! => allows us to rename on import to avoid clashes + ! "o_" for original (TODO: better naming convention) + use m_error_v_passing, only: & + o_pass_error => pass_error, & + o_pass_errors => pass_errors + use m_error_v, only: ErrorV + + ! The manager module, which makes this all work + use m_error_v_manager, only: & + error_v_manager_get_instance => get_instance + ! error_v_manager_get_available_instance_index => get_available_instance_index, & + ! error_v_manager_set_instance_index_to => set_instance_index_to, & + ! error_v_manager_ensure_instance_array_size_is_at_least => ensure_instance_array_size_is_at_least + + implicit none + private + + public :: pass_error, pass_errors + +contains + + function pass_error(inv_instance_index) result(res) + !! Wrapper around `m_error_v_passing.pass_error` (TODO: x-ref) + + integer, intent(in) :: inv_instance_index + !! Input values + !! + !! See docstring of `m_error_v_passing.pass_error` for details. + !! [TODO: x-ref] + !! The trick here is to pass in the instance index, not the instance itself + + logical :: res + !! Whether the instance referred to by `inv_instance_index` is an error or not + + type(ErrorV) :: instance + + instance = error_v_manager_get_instance(inv_instance_index) + + ! Do the Fortran call + res = o_pass_error(instance) + + end function pass_error + + function pass_errors(inv_instance_indexes, n) result(res) + !! Wrapper around `m_error_v_passing.pass_errors` (TODO: x-ref) + + integer, dimension(n), intent(in) :: inv_instance_indexes + !! Input values + !! + !! See docstring of `m_error_v_passing.pass_errors` for details. + !! [TODO: x-ref] + + integer, intent(in) :: n + !! Number of values to pass + + logical, dimension(n) :: res + !! Whether each instance in the array backed by `inv_instance_indexes` is an error or not + ! + ! This is the major trick for wrapping. + ! We pass instance indexes (integers) from Python rather than the instance itself. + + type(ErrorV), dimension(n) :: instances + + integer :: i + + do i = 1, n + instances(i) = error_v_manager_get_instance(inv_instance_indexes(i)) + end do + + ! Do the Fortran call + res = o_pass_errors(instances, n) + + end function pass_errors + +end module m_error_v_passing_w diff --git a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py b/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py deleted file mode 100644 index dab17f0..0000000 --- a/src/example_fgen_basic/pyfgen_runtime/base_finalisable.py +++ /dev/null @@ -1,218 +0,0 @@ -""" -Runtime helper to support wrapping of Fortran derived types -""" - -from __future__ import annotations - -from abc import ABC, abstractmethod -from functools import wraps -from typing import Any, Callable, Concatenate, ParamSpec, TypeVar - -import attrs -from attrs import define, field - -from example_fgen_basic.pyfgen_runtime.exceptions import ( - NotInitialisedError, -) -from example_fgen_basic.pyfgen_runtime.formatting import to_html, to_pretty, to_str - -# Might be needed for Python 3.9 -# from typing_extensions import Concatenate, ParamSpec - - -INVALID_INSTANCE_INDEX: int = -1 -""" -Value used to denote an invalid `instance_index`. - -This can occur value when a wrapper class -has not yet been initialised (connected to a Fortran instance). -""" - - -@define -class FinalisableWrapperBase(ABC): - """ - Base class for Fortran derived type wrappers - """ - - instance_index: int = field( - validator=attrs.validators.instance_of(int), - default=INVALID_INSTANCE_INDEX, - ) - """ - Model index of wrapper Fortran instance - """ - - def __str__(self) -> str: - """ - Get string representation of self - """ - return to_str( - self, - self.exposed_attributes, - ) - - def _repr_pretty_(self, p: Any, cycle: bool) -> None: - """ - Get pretty representation of self - - Used by IPython notebooks and other tools - """ - to_pretty( - self, - self.exposed_attributes, - p=p, - cycle=cycle, - ) - - def _repr_html_(self) -> str: - """ - Get html representation of self - - Used by IPython notebooks and other tools - """ - return to_html( - self, - self.exposed_attributes, - ) - - @property - def initialised(self) -> bool: - """ - Is the instance initialised, i.e. connected to a Fortran instance? - """ - return self.instance_index != INVALID_INSTANCE_INDEX - - @property - @abstractmethod - def exposed_attributes(self) -> tuple[str, ...]: - """ - Attributes exposed by this wrapper - """ - ... - - # TODO: consider whether we need these - # @classmethod - # @abstractmethod - # def from_new_connection(cls) -> FinalisableWrapperBase: - # """ - # Initialise by establishing a new connection with the Fortran module - # - # This requests a new model index from the Fortran module and then - # initialises a class instance - # - # Returns - # ------- - # New class instance - # """ - # ... - # - # @abstractmethod - # def finalise(self) -> None: - # """ - # Finalise the Fortran instance and set self back to being uninitialised - # - # This method resets ``self.instance_index`` back to - # ``_UNINITIALISED_instance_index`` - # - # Should be decorated with :func:`check_initialised` - # """ - # # call to Fortran module goes here when implementing - # self._uninitialise_instance_index() - - def _uninitialise_instance_index(self) -> None: - self.instance_index = INVALID_INSTANCE_INDEX - - -P = ParamSpec("P") -T = TypeVar("T") -Wrapper = TypeVar("Wrapper", bound=FinalisableWrapperBase) - - -def check_initialised( - method: Callable[Concatenate[Wrapper, P], T], -) -> Callable[Concatenate[Wrapper, P], T]: - """ - Check that the wrapper object has been initialised before executing the method - - Parameters - ---------- - method - Method to wrap - - Returns - ------- - : - Wrapped method - - Raises - ------ - InitialisationError - Wrapper is not initialised - """ - - @wraps(method) - def checked( - ref: Wrapper, - *args: P.args, - **kwargs: P.kwargs, - ) -> Any: - if not ref.initialised: - raise NotInitialisedError(ref, method) - - return method(ref, *args, **kwargs) - - return checked # type: ignore - - -# Thank you for type hints info -# https://adamj.eu/tech/2021/05/11/python-type-hints-args-and-kwargs/ -def execute_finalise_on_fail( - inst: FinalisableWrapperBase, - func_to_try: Callable[Concatenate[int, P], T], - *args: P.args, - **kwargs: P.kwargs, -) -> T: - """ - Execute a function, finalising the instance before raising if any error occurs - - This function is most useful in factory functions where it provides a - clean way of ensuring that any Fortran is freed up in the event of an - initialisation failure for any reason - - @Marco note: I'm not sure this is a very good pattern, - we may get rid of it. - - Parameters - ---------- - inst - Instance whose model index we will use when executing the functin - - func_to_try - Function to try executing, must take `inst`'s instance pointer as its - first argument - - *args - Passed to `func_to_try` - - **kwargs - Passed to `func_to_try` - - Returns - ------- - : - Result of calling `func_to_try(inst.instance_ptr, *args, **kwargs)` - - Raises - ------ - Exception - Any exception which occurs when calling `func_to_try`. Before the - exception is raised, `inst.finalise()` is called. - """ - try: - return func_to_try(inst.instance_index, *args, **kwargs) - except RuntimeError: - # finalise the instance before raising - inst.finalise() - - raise diff --git a/src/example_fgen_basic/pyfgen_runtime/formatting.py b/src/example_fgen_basic/pyfgen_runtime/formatting.py deleted file mode 100644 index 7245121..0000000 --- a/src/example_fgen_basic/pyfgen_runtime/formatting.py +++ /dev/null @@ -1,234 +0,0 @@ -""" -Formatting support - -I.e. display of wrapped Fortran derived types in a nice way -""" - -from __future__ import annotations - -from collections.abc import Iterable -from typing import TYPE_CHECKING, Any - -from example_fgen_basic.pyfgen_runtime.exceptions import UnallocatedMemoryError - -# Might be needed for Python 3.9 -# from typing_extensions import Concatenate, ParamSpec -if TYPE_CHECKING: - from example_fgen_basic.pyfgen_runtime.base_finalisable import ( - FinalisableWrapperBase, - ) - - -def get_attribute_str_value(instance: FinalisableWrapperBase, attribute: str) -> str: - """ - Get the string version of an attribute's value - - Parameters - ---------- - instance - Instance from which to get the attribute - - attribute - Attribute for which to get the value - - Returns - ------- - String version of the attribute's value, with graceful handling of errors. - """ - try: - return f"{attribute}={getattr(instance, attribute)}" - except UnallocatedMemoryError: - # TODO: change this when we move to better error handling - return f"{attribute} is unallocated" - - -def to_str(instance: FinalisableWrapperBase, exposed_attributes: Iterable[str]) -> str: - """ - Convert an instance to its string representation - - Parameters - ---------- - instance - Instance to convert - - exposed_attributes - Attributes from Fortran that the instance exposes - - Returns - ------- - String representation of the instance - """ - if not instance.initialised: - return f"Uninitialised {instance!r}" - - if not exposed_attributes: - return repr(instance) - - attribute_values = [ - get_attribute_str_value(instance, v) for v in exposed_attributes - ] - - return f"{repr(instance)[:-1]}, {', '.join(attribute_values)})" - - -def to_pretty( - instance: FinalisableWrapperBase, - exposed_attributes: Iterable[str], - p: Any, - cycle: bool, - indent: int = 4, -) -> None: - """ - Pretty-print an instance - - Parameters - ---------- - instance - Instance to convert - - exposed_attributes - Attributes from Fortran that the instance exposes - - p - Pretty printing object - - cycle - Whether the pretty printer has detected a cycle or not. - - indent - Indent to apply to the pretty printing group - """ - if not instance.initialised: - p.text(str(instance)) - return - - if not exposed_attributes: - p.text(str(instance)) - return - - with p.group(indent, f"{repr(instance)[:-1]}", ")"): - for att in exposed_attributes: - p.text(",") - p.breakable() - - p.text(get_attribute_str_value(instance, att)) - - -def add_attribute_row( - attribute_name: str, attribute_value: str, attribute_rows: list[str] -) -> list[str]: - """ - Add a row for displaying an attribute's value to a list of rows - - Parameters - ---------- - attribute_name - Attribute's name - - attribute_value - Attribute's value - - attribute_rows - Existing attribute rows - - - Returns - ------- - Attribute rows, with the new row appended - """ - attribute_rows.append( - f"" # noqa: E501 - ) - - return attribute_rows - - -def to_html(instance: FinalisableWrapperBase, exposed_attributes: Iterable[str]) -> str: - """ - Convert an instance to its html representation - - Parameters - ---------- - instance - Instance to convert - - exposed_attributes - Attributes from Fortran that the instance exposes - - Returns - ------- - HTML representation of the instance - """ - if not instance.initialised: - return str(instance) - - if not exposed_attributes: - return str(instance) - - instance_class_name = repr(instance).split("(")[0] - - attribute_rows: list[str] = [] - for att in exposed_attributes: - try: - att_val = getattr(instance, att) - except UnallocatedMemoryError: - # TODO: change this when we move to better error handling - att_val = "Unallocated" - attribute_rows = add_attribute_row(att, att_val, attribute_rows) - continue - - try: - att_val = att_val._repr_html_() - except AttributeError: - att_val = str(att_val) - - attribute_rows = add_attribute_row(att, att_val, attribute_rows) - - attribute_rows_for_table = "\n ".join(attribute_rows) - - css_style = """.fgen-wrap { - /*font-family: monospace;*/ - width: 540px; -} - -.fgen-header { - padding: 6px 0 6px 3px; - border-bottom: solid 1px #777; - color: #555;; -} - -.fgen-header > div { - display: inline; - margin-top: 0; - margin-bottom: 0; -} - -.fgen-basefinalizable-cls, -.fgen-basefinalizable-instance-index { - margin-left: 2px; - margin-right: 10px; -} - -.fgen-basefinalizable-cls { - font-weight: bold; - color: #000000; -}""" - - return "\n".join( - [ - "
", - " ", - "
", - "
", - f"
{instance_class_name}
", - f"
instance_index={instance.instance_index}
", # noqa: E501 - "
code0
message
{attribute_name}{attribute_value}
", - f" {attribute_rows_for_table}", - "
", - "
", - "
", - "", - ] - ) diff --git a/tests/unit/test_error_v_creation.f90 b/tests/unit/test_error_v_creation.f90 index 61e3ce5..7d0391e 100644 --- a/tests/unit/test_error_v_creation.f90 +++ b/tests/unit/test_error_v_creation.f90 @@ -27,7 +27,7 @@ end subroutine collect_error_v_creation_tests subroutine test_error_v_creation_basic(error) use m_error_v, only: ErrorV - use m_error_v_creation, only: create_error + use m_error_v_passing, only: create_error type(error_type), allocatable, intent(out) :: error @@ -46,7 +46,7 @@ end subroutine test_error_v_creation_basic subroutine test_error_v_creation_edge(error) use m_error_v, only: ErrorV - use m_error_v_creation, only: create_error + use m_error_v_passing, only: create_error type(error_type), allocatable, intent(out) :: error diff --git a/tests/unit/test_error_v_passing.py b/tests/unit/test_error_v_passing.py index 6311dfe..341ac00 100644 --- a/tests/unit/test_error_v_passing.py +++ b/tests/unit/test_error_v_passing.py @@ -27,11 +27,11 @@ def test_pass_error_lots_of_repeated_calls(): # Fortran derived types correctly # (and sort of a speed test, this shouldn't be noticeably slow) # hence we may move this test somewhere more generic at some point. - for _ in range(1e5): + for _ in range(int(1e5)): pass_error(ErrorV(code=0)) def test_pass_multiple_errors(): res = pass_errors([ErrorV(code=0), ErrorV(code=0), ErrorV(code=1)]) - np.testing.assert_all_equal(res, np.array([True, True, False])) + np.testing.assert_array_equal(res, np.array([False, False, True])) From 801c9450c7bd75b48fbb7c33e3f4ace311e74667 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 29 Aug 2025 15:48:16 +0200 Subject: [PATCH 24/49] Add typing module and TODOs --- TODO.md | 8 ++++ meson.build | 1 + pyproject.toml | 2 +- src/example_fgen_basic/error_v/creation.py | 3 +- .../error_v/error_v_manager.f90 | 2 - src/example_fgen_basic/error_v/passing.py | 4 +- src/example_fgen_basic/typing.py | 43 +++++++++++++++++++ 7 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 TODO.md create mode 100644 src/example_fgen_basic/typing.py diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..fbadc46 --- /dev/null +++ b/TODO.md @@ -0,0 +1,8 @@ +- read through everything + - check docstrings + - check naming (if you want to propose a consistent naming convention, go for it) +- consider adding `m_error_v_w.build_instances` too, might be headache to pass arrays of things up and down correctly +- clean up +- final review (including deleting this file) +- merge +- move onto a basic result type with a subclass specifically for integers (`ResultInt` or something) diff --git a/meson.build b/meson.build index d9e0e4b..7561b37 100644 --- a/meson.build +++ b/meson.build @@ -82,6 +82,7 @@ if pyprojectwheelbuild_enabled 'src/example_fgen_basic/get_wavelength.py', 'src/example_fgen_basic/pyfgen_runtime/__init__.py', 'src/example_fgen_basic/pyfgen_runtime/exceptions.py', + 'src/example_fgen_basic/typing.py', ) # The ancillary library, diff --git a/pyproject.toml b/pyproject.toml index 0fa52ab..6a76447 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -149,7 +149,7 @@ source = [ ] branch = true omit = [ - # TODO: check this file + # TODO: test these files directly when splitting out pyfgen_runtime "*exceptions.py", "*runtime_helpers.py", ] diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py index 3e18787..b53a5b7 100644 --- a/src/example_fgen_basic/error_v/creation.py +++ b/src/example_fgen_basic/error_v/creation.py @@ -28,8 +28,7 @@ ) from exc if TYPE_CHECKING: - # TODO: bring back in numpy type hints - NP_ARRAY_OF_INT = None + from example_fgen_basic.typing import NP_ARRAY_OF_INT def create_error(inv: int) -> ErrorV: diff --git a/src/example_fgen_basic/error_v/error_v_manager.f90 b/src/example_fgen_basic/error_v/error_v_manager.f90 index 34bfa53..6956a92 100644 --- a/src/example_fgen_basic/error_v/error_v_manager.f90 +++ b/src/example_fgen_basic/error_v/error_v_manager.f90 @@ -2,8 +2,6 @@ !> !> Written by hand here. !> Generation to be automated in future (including docstrings of some sort). -! -! TODO: make it possible to reallocate the number of instances module m_error_v_manager use m_error_v, only: ErrorV diff --git a/src/example_fgen_basic/error_v/passing.py b/src/example_fgen_basic/error_v/passing.py index 4f832ae..a6d9bff 100644 --- a/src/example_fgen_basic/error_v/passing.py +++ b/src/example_fgen_basic/error_v/passing.py @@ -30,9 +30,7 @@ ) from exc if TYPE_CHECKING: - # TODO: bring back in numpy type hints - NP_ARRAY_OF_INT = None - NP_ARRAY_OF_BOOL = None + from example_fgen_basic.typing import NP_ARRAY_OF_BOOL, NP_ARRAY_OF_INT def pass_error(inv: ErrorV) -> bool: diff --git a/src/example_fgen_basic/typing.py b/src/example_fgen_basic/typing.py new file mode 100644 index 0000000..f71051d --- /dev/null +++ b/src/example_fgen_basic/typing.py @@ -0,0 +1,43 @@ +""" +Type hints which are too annoying to remember +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Any, TypeAlias, Union + + import numpy as np + import numpy.typing as npt + + NP_ARRAY_OF_INT: TypeAlias = npt.NDArray[np.integer] + """ + Type alias for an array of numpy int + """ + + NP_ARRAY_OF_FLOAT: TypeAlias = npt.NDArray[np.floating] + """ + Type alias for an array of numpy floats + """ + + NP_FLOAT_OR_INT: TypeAlias = Union[np.floating, np.integer] + """ + Type alias for a numpy float or int (not complex) + """ + + NP_ARRAY_OF_FLOAT_OR_INT: TypeAlias = npt.NDArray[NP_FLOAT_OR_INT] + """ + Type alias for an array of numpy float or int (not complex) + """ + + NP_ARRAY_OF_NUMBER: TypeAlias = npt.NDArray[np.number[Any]] + """ + Type alias for an array of numpy float or int (including complex) + """ + + NP_ARRAY_OF_BOOL: TypeAlias = npt.NDArray[np.bool] + """ + Type alias for an array of booleans + """ From 11b9409feea26fc551099f49345f13bd81c2aba6 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Sun, 31 Aug 2025 13:17:35 +0200 Subject: [PATCH 25/49] Update TODO --- TODO.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/TODO.md b/TODO.md index fbadc46..5728c19 100644 --- a/TODO.md +++ b/TODO.md @@ -5,4 +5,5 @@ - clean up - final review (including deleting this file) - merge -- move onto a basic result type with a subclass specifically for integers (`ResultInt` or something) +- move onto a basic result type with a subclass specifically for integers + (`ResultInt` or something, outline here: https://github.com/openscm/example-fgen-basic/tree/result-type) From 0ef5506b7453f3d3159f2f7dda40a62a9984a4e5 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Wed, 3 Sep 2025 12:58:40 +0200 Subject: [PATCH 26/49] Add comment --- src/example_fgen_basic/error_v/error_v_wrapper.f90 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/example_fgen_basic/error_v/error_v_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_wrapper.f90 index 7cde3a0..893a86d 100644 --- a/src/example_fgen_basic/error_v/error_v_wrapper.f90 +++ b/src/example_fgen_basic/error_v/error_v_wrapper.f90 @@ -44,6 +44,12 @@ subroutine build_instance(code, message, instance_index) end subroutine build_instance + ! build_instances is very hard to do + ! because you need to pass an array of variable-length characters which is non-trivial. + ! Maybe we will try this another day, for now this isn't that important + ! (we can just use a loop from the Python side) + ! so we just don't bother implementing `build_instances`. + subroutine finalise_instance(instance_index) !! Finalise an instance From bc54ccf31642f85590cfe2132fce7a2d24bffaab Mon Sep 17 00:00:00 2001 From: Marco Zecchetto Date: Thu, 4 Sep 2025 18:38:41 +0200 Subject: [PATCH 27/49] Some clean up --- src/example_fgen_basic/error_v/creation.f90 | 2 -- src/example_fgen_basic/error_v/passing.f90 | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/example_fgen_basic/error_v/creation.f90 b/src/example_fgen_basic/error_v/creation.f90 index 50935ed..aa54681 100644 --- a/src/example_fgen_basic/error_v/creation.f90 +++ b/src/example_fgen_basic/error_v/creation.f90 @@ -2,8 +2,6 @@ !> !> A very basic demo to get the idea. ! -! TODO: discuss - we should probably have some convention for module names. -! The hard part is avoiding them becoming too long... module m_error_v_creation use m_error_v, only: ErrorV, NO_ERROR_CODE diff --git a/src/example_fgen_basic/error_v/passing.f90 b/src/example_fgen_basic/error_v/passing.f90 index be0fc9a..eb44274 100644 --- a/src/example_fgen_basic/error_v/passing.f90 +++ b/src/example_fgen_basic/error_v/passing.f90 @@ -2,8 +2,6 @@ !> !> A very basic demo to get the idea. ! -! TODO: discuss - we should probably have some convention for module names. -! The hard part is avoiding them becoming too long... module m_error_v_passing use m_error_v, only: ErrorV, NO_ERROR_CODE From cc8637c8c7304c26eb7264512b5d9422e0ec14a8 Mon Sep 17 00:00:00 2001 From: Marco Zecchetto Date: Thu, 4 Sep 2025 18:44:10 +0200 Subject: [PATCH 28/49] Added changelog --- changelog/30.trivial.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/30.trivial.md diff --git a/changelog/30.trivial.md b/changelog/30.trivial.md new file mode 100644 index 0000000..440b804 --- /dev/null +++ b/changelog/30.trivial.md @@ -0,0 +1 @@ +- Made some clean up From e6b58692491fd0ab758002667e2cc3b038f9cb80 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 12:35:19 +0200 Subject: [PATCH 29/49] Update docs --- docs/NAVIGATION.md | 3 +- .../wrapping-fortran-derived-types.md | 57 +++++++++---------- docs/how-to-guides/basic-calculation.md | 10 ---- docs/how-to-guides/index.md | 12 ---- docs/how-to-guides/run-code-in-a-notebook.py | 24 -------- src/example_fgen_basic/error_v/creation.py | 2 +- 6 files changed, 29 insertions(+), 79 deletions(-) rename IMPLEMENTATION-NOTES.md => docs/further-background/wrapping-fortran-derived-types.md (85%) delete mode 100644 docs/how-to-guides/basic-calculation.md delete mode 100644 docs/how-to-guides/run-code-in-a-notebook.py diff --git a/docs/NAVIGATION.md b/docs/NAVIGATION.md index fb48646..1646fa4 100644 --- a/docs/NAVIGATION.md +++ b/docs/NAVIGATION.md @@ -6,11 +6,10 @@ See https://oprypin.github.io/mkdocs-literate-nav/ - [Home](index.md) - [Installation](installation.md) - [How-to guides](how-to-guides/index.md) - - [Do a basic calculation](how-to-guides/basic-calculation.md) - - [Run code in a notebook](how-to-guides/run-code-in-a-notebook.py) - [Tutorials](tutorials/index.md) - [Further background](further-background/index.md) - [Dependency pinning and testing](further-background/dependency-pinning-and-testing.md) + - [Wrapping Fortran derived types](further-background/wrapping-fortran-derived-types.md) - [Development](development.md) - [API reference](api/example_fgen_basic/) - [Fortran API](fortran-api/home.html) diff --git a/IMPLEMENTATION-NOTES.md b/docs/further-background/wrapping-fortran-derived-types.md similarity index 85% rename from IMPLEMENTATION-NOTES.md rename to docs/further-background/wrapping-fortran-derived-types.md index e939a42..1487621 100644 --- a/IMPLEMENTATION-NOTES.md +++ b/docs/further-background/wrapping-fortran-derived-types.md @@ -1,4 +1,6 @@ -@Marco please move these into `docs/further-background/wrapping-derived-types.md` (and do any other clean up and formatting fixes you'd like) +# Wrapping derived types + +Here we describe our approach to wrapping Fortran derived types. ## What is the goal? @@ -30,7 +32,7 @@ the original Python object will remain unchanged This assumption makes ownership and memory management clear. We do not need to keep instances around as views -and worry about consistency across the Python-Fortran interface. +and therefore do not need to worry about consistency across the Python-Fortran interface. Instead, we simply pass data back and forth, and the normal rules of data consistency within each programming language apply. @@ -45,57 +47,54 @@ The manager module has two key components: In practice, they are essentially temporary variables. 1. an allocatable array of logical (boolean) values, call this `available_array`. - The convention is that, if `available_array(i)`, where `i` is an integer, - is `.true.` then the instance at `instance_array(i)` is available for the manager to use, - otherwise the manager assumes that the instance is already being used for some purpose + The convention is that, if `available_array(i)` is `.true.`, + where `i` is an integer, + then the instance at `instance_array(i)` is available for the manager to use. + Otherwise, the manager assumes that the instance is already being used for some purpose and therefore cannot be used for whatever operation is currently being performed. This setup allows us to effectively pass derived types back and forth between Python and Fortran. -Whenever we need to return a derived type to Python, we: - -[TODO think about retrieving multiple derived types at once] +Whenever we need to return a derived type (or derived types) to Python, we: -1. get the derived type from whatever Fortran function or subroutine created it, +1. get the derived type(s) from whatever Fortran function or subroutine created it, call this `derived_type_original` 1. find an index, `idx`, in `available_array` such that `available_array(idx)` is `.true.` 1. set `instance_array(idx)` equal to `derived_type_original` 1. we return `idx` to Python - - `idx` is an integer, so we can return this easily to Python using `f2py` -1. we then create a Python object with an API that mirrors `derived_type_original` + - `idx` is an integer (or integers), so we can return this easily to Python using `f2py` +1. we then create a Python object (or objects) with an API that mirrors `derived_type_original` using the class method `from_instance_index`. - This class method is [TODO or will be] auto-generated via `pyfgen` + This class method is auto-generated via `pyfgen` + (TODO: implement auto-generation) and handles retrieval of all the attribute values of `derived_type_original` from Fortran and sets them on the Python object that is being instantiated - we can do this as, if you dig down deep enough, all attributes eventually become primitive types which can be passed back and forth using `f2py`, it can just be that multiple levels of recursion are needed if you have derived types that themselves have derived type attributes -1. we then call the manager [TODO I think this will end up being wrapper, we can tighten the language later] - module's `finalise_instance` function to free the (temporary) instance +1. we then call the wrapper module's `finalise_instance` function to free the (temporary) instance(s) that was used by the manager + (we cannot access the manager module directly as it cannot be wrapped by `f2py`) - this instance is no longer needed because all the data has been transferred to Python -1. we end up with a Python instance that has the result +1. we end up with a Python instance(s) that has the result and no extra/leftover memory footprint in Fortran (and leave Fortran to decide whether to clean up `derived_type_original` or not) -Whenever we need to pass a derived type to Fortran, we: - -[TODO think about passing multiple derived types at once] +Whenever we need to pass a derived type (or derived types) to Fortran, we: -1. call the manager [TODO I think this will end up being wrapper, we can tighten the language later] - module's `get_free_instance_index` function to get an available index to use for the passing -1. call the manager [TODO I think this will end up being wrapper, we can tighten the language later] - module's `build_instance` function with the index we just received - plus all of the Python object's attribute values - - on the Fortran side, there is now an instantiated derived type, ready for use +1. get an instance index we can use to communicate with Fortran + 1. call the Python object's `build_fortran_instance` method, + which returns the instance index where the object was created on the Fortran side. + Under the hood, this calls the wrapper module's + `get_free_instance_index` and `build_instance` functions + 1. on the Fortran side, there is now an instantiated derived type, ready for use 1. call the wrapped Fortran function of interest, - except we pass the instance index instead of the derived type + except we pass the instance index instead of the actual Python object itself 1. on the Fortran side, retrieve the instantiated index from the manager module and use this to call the Fortran function/subroutine of interest 1. return the result from Fortran back to Python -1. call the manager [TODO I think this will end up being wrapper, we can tighten the language later] - module's `finalise_instance` function to free the (temporary) instance +1. call the wrapper module's `finalise_instance` function to free the (temporary) instance that was used to pass the instance in the first place - this instance is no longer needed because all the data has been transferred and used by Fortran 1. we end up with the result of the Fortran callable back in Python @@ -175,9 +174,7 @@ and normal Python rules apply in Python Note: this section was never properly finished. Once we started trying to write it, we realised how hard it would be to avoid weird edge cases -so we stopped and changed to [our current solution][Our solution] -(@Marco please check that this internal cross-reference works -once the docs are built). +so we stopped and changed to [our current solution][our-solution]. To pass derived types back and forth across the Python-Fortran interface, we introduce a 'manager' module for all derived types. diff --git a/docs/how-to-guides/basic-calculation.md b/docs/how-to-guides/basic-calculation.md deleted file mode 100644 index cbe227a..0000000 --- a/docs/how-to-guides/basic-calculation.md +++ /dev/null @@ -1,10 +0,0 @@ -# How to do a basic calculation - -An example of how to do a basic calculation using Example fgen - basic. - -```python ->>> from example_fgen_basic.operations import add_two - ->>> add_two(3.2, 4.3) -7.5 -``` diff --git a/docs/how-to-guides/index.md b/docs/how-to-guides/index.md index 76c5267..ea58c94 100644 --- a/docs/how-to-guides/index.md +++ b/docs/how-to-guides/index.md @@ -3,15 +3,3 @@ This part of the project documentation focuses on a **problem-oriented** approach. We'll go over how to solve common tasks. - -## How can I do a basic calculation? - - - -If you want to do a basic calculation, -see ["How to do a basic calculation"][how-to-do-a-basic-calculation]. diff --git a/docs/how-to-guides/run-code-in-a-notebook.py b/docs/how-to-guides/run-code-in-a-notebook.py deleted file mode 100644 index 6166066..0000000 --- a/docs/how-to-guides/run-code-in-a-notebook.py +++ /dev/null @@ -1,24 +0,0 @@ -# --- -# jupyter: -# jupytext: -# text_representation: -# extension: .py -# format_name: percent -# format_version: '1.3' -# jupytext_version: 1.16.4 -# kernelspec: -# display_name: Python 3 (ipykernel) -# language: python -# name: python3 -# --- - -# %% [markdown] editable=true slideshow={"slide_type": ""} -# # How to run code in a notebook -# -# Here we show how you can make your docs via notebooks. - -# %% editable=true slideshow={"slide_type": ""} -from example_fgen_basic.operations import add_two - -# %% editable=true slideshow={"slide_type": ""} -add_two(3.2, 4.3) diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py index b53a5b7..bcc67af 100644 --- a/src/example_fgen_basic/error_v/creation.py +++ b/src/example_fgen_basic/error_v/creation.py @@ -41,7 +41,7 @@ def create_error(inv: int) -> ErrorV: Input value If odd, the error code is - [NO_ERROR_CODE][example_fgen_basic.error_v.NO_ERROR_CODE]. + [NO_ERROR_CODE][example_fgen_basic.error_v.error_v.NO_ERROR_CODE]. If even, the error code is 1. If a negative number is supplied, the error code is 2. From 2d95437e33b275f4fa25fd257ed90e9ed9f75979 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 12:35:58 +0200 Subject: [PATCH 30/49] Remove changelog entry that isn't needed --- changelog/30.trivial.md | 1 - 1 file changed, 1 deletion(-) delete mode 100644 changelog/30.trivial.md diff --git a/changelog/30.trivial.md b/changelog/30.trivial.md deleted file mode 100644 index 440b804..0000000 --- a/changelog/30.trivial.md +++ /dev/null @@ -1 +0,0 @@ -- Made some clean up From 081f138aa271efb908083a1a091fa4232c5ed0ef Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 13:51:56 +0200 Subject: [PATCH 31/49] Add basic demo back in --- docs/NAVIGATION.md | 1 + docs/how-to-guides/basic-demo.py | 82 +++++++++++++++++++++++ pyproject.toml | 1 + requirements-docs-locked.txt | 3 + src/example_fgen_basic/error_v/error_v.py | 18 +++++ src/example_fgen_basic/error_v/passing.py | 2 +- uv.lock | 2 + 7 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 docs/how-to-guides/basic-demo.py diff --git a/docs/NAVIGATION.md b/docs/NAVIGATION.md index 1646fa4..eb9e819 100644 --- a/docs/NAVIGATION.md +++ b/docs/NAVIGATION.md @@ -6,6 +6,7 @@ See https://oprypin.github.io/mkdocs-literate-nav/ - [Home](index.md) - [Installation](installation.md) - [How-to guides](how-to-guides/index.md) + - [Basic demo](how-to-guides/basic-demo.py) - [Tutorials](tutorials/index.md) - [Further background](further-background/index.md) - [Dependency pinning and testing](further-background/dependency-pinning-and-testing.md) diff --git a/docs/how-to-guides/basic-demo.py b/docs/how-to-guides/basic-demo.py new file mode 100644 index 0000000..1009b1e --- /dev/null +++ b/docs/how-to-guides/basic-demo.py @@ -0,0 +1,82 @@ +# --- +# jupyter: +# jupytext: +# text_representation: +# extension: .py +# format_name: percent +# format_version: '1.3' +# jupytext_version: 1.17.2 +# kernelspec: +# display_name: Python 3 (ipykernel) +# language: python +# name: python3 +# --- + +# %% [markdown] +# # Basic demo +# +# Here we show a very basic demo of how to use the package. +# The actual behaviour isn't so interesting, +# but it demonstrates that we can wrap code +# that is ultimately written in Fortran. + +# %% [markdown] +# ## Imports + +# %% +import pint + +from example_fgen_basic.error_v import ErrorV +from example_fgen_basic.error_v.creation import create_error, create_errors +from example_fgen_basic.error_v.passing import pass_error, pass_errors +from example_fgen_basic.get_wavelength import get_wavelength, get_wavelength_plain + +# %% [markdown] +# ## Calculation with basic types +# +# Here we show how we can use a basic wrapped function. +# This functionality isn't actually specific to our wrappers +# (you can do the same with f2py), +# but it's a useful starting demonstration. + +# %% +# `_plain` because this works on plain floats, +# not quantities with units (see below for this demonstration) +get_wavelength_plain(400.0e12) + +# %% [markdown] +# With these python wrappers, +# we can also do nice things like support interfaces that use units +# (this would be much more work to implement directly in Fortran). + +# %% +ur = pint.get_application_registry() + +# %% +get_wavelength(ur.Quantity(400.0, "THz")).to("nm") + +# %% [markdown] +# ## Receiving and passing derived types +# +# TODO: more docs and cross-references on how this actually works + +# %% [markdown] +# We can receive a Python-equivalent of a Fortran derived type. + +# %% +create_error(3) + +# %% [markdown] +# Or multiple derived types. + +# %% +create_errors([1, 2, 1, 5]) + +# %% [markdown] +# We can also pass Python-equivalent of Fortran derived types back into Fortran. + +# %% +pass_error(ErrorV(code=0)) + +# %% +pass_errors([ErrorV(code=0), ErrorV(code=3), ErrorV(code=5), ErrorV(code=-2)]) diff --git a/pyproject.toml b/pyproject.toml index 6a76447..acdf664 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,6 +94,7 @@ docs = [ "jupyterlab>=4.4.5", "jupytext>=1.17.2", "mkdocs-jupyter>=0.25.1", + "pint>=0.24.4", ] # For minimum test dependencies. # These are used when running our minimum PyPI install tests. diff --git a/requirements-docs-locked.txt b/requirements-docs-locked.txt index 1eb5ec2..98adf97 100644 --- a/requirements-docs-locked.txt +++ b/requirements-docs-locked.txt @@ -28,6 +28,8 @@ defusedxml==0.7.1 exceptiongroup==1.3.0 ; python_full_version < '3.11' executing==2.1.0 fastjsonschema==2.21.1 +flexcache==0.3 +flexparser==0.4 fonttools==4.59.0 ford==6.1.13 fqdn==1.5.1 @@ -103,6 +105,7 @@ parso==0.8.4 pathspec==0.12.1 pexpect==4.9.0 ; (python_full_version < '3.10' and sys_platform == 'emscripten') or (sys_platform != 'emscripten' and sys_platform != 'win32') pillow==11.3.0 +pint==0.24.4 platformdirs==4.3.6 prometheus-client==0.21.1 prompt-toolkit==3.0.48 diff --git a/src/example_fgen_basic/error_v/error_v.py b/src/example_fgen_basic/error_v/error_v.py index 7fc12c8..c508148 100644 --- a/src/example_fgen_basic/error_v/error_v.py +++ b/src/example_fgen_basic/error_v/error_v.py @@ -60,3 +60,21 @@ def from_instance_index(cls, instance_index: int) -> ErrorV: res = cls(code=code, message=message) return res + + def build_fortran_instance(self) -> int: + """ + Build an instance equivalent to `self` on the Fortran side + + Intended for use mainly by wrapping functions. + Most users should not need to use this method directly. + + Returns + ------- + : + Instance index of the object which has been created on the Fortran side + """ + instance_index: int = m_error_v_w.build_instance( + code=self.code, message=self.message + ) + + return instance_index diff --git a/src/example_fgen_basic/error_v/passing.py b/src/example_fgen_basic/error_v/passing.py index a6d9bff..bea4642 100644 --- a/src/example_fgen_basic/error_v/passing.py +++ b/src/example_fgen_basic/error_v/passing.py @@ -48,7 +48,7 @@ def pass_error(inv: ErrorV) -> bool: If `inv` is an error, `True`, otherwise `False`. """ # Tell Fortran to build the object on the Fortran side - instance_index: int = m_error_v_w.build_instance(code=inv.code, message=inv.message) + instance_index = inv.build_fortran_instance() # Call the Fortran function # Boolean wrapping strategy, have to cast to bool diff --git a/uv.lock b/uv.lock index 5833384..3a56b43 100644 --- a/uv.lock +++ b/uv.lock @@ -825,6 +825,7 @@ docs = [ { name = "mkdocs-section-index" }, { name = "mkdocstrings-python" }, { name = "mkdocstrings-python-xref" }, + { name = "pint" }, { name = "pymdown-extensions" }, { name = "ruff" }, ] @@ -917,6 +918,7 @@ docs = [ { name = "mkdocs-section-index", specifier = ">=0.3.10" }, { name = "mkdocstrings-python", specifier = ">=1.16.12" }, { name = "mkdocstrings-python-xref", specifier = ">=1.16.3" }, + { name = "pint", specifier = ">=0.24.4" }, { name = "pymdown-extensions", specifier = ">=10.16.1" }, { name = "ruff", specifier = ">=0.12.8" }, ] From 09e99c7c2a254e1995be06b096d0bad05cb1d0d0 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 13:54:02 +0200 Subject: [PATCH 32/49] Delete unneeded print statement --- meson.build | 1 - 1 file changed, 1 deletion(-) diff --git a/meson.build b/meson.build index 7561b37..84134c5 100644 --- a/meson.build +++ b/meson.build @@ -149,7 +149,6 @@ if pyprojectwheelbuild_enabled # this is where that operation happens. # Files get copied to /site-packages/ foreach python_src : python_srcs - message(fs.relative_to(python_src, 'src' / python_project_name)) py.install_sources( python_src, subdir: fs.parent(fs.relative_to(python_src, 'src')), From 4d3402948eaba54e41ae02655c89b89519f6e2a3 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 13:55:02 +0200 Subject: [PATCH 33/49] Remove unneded files --- .../test_error_html.html | 41 ------------------- .../test_error_pprint.txt | 1 - .../test_error_v_creation/test_error_str.txt | 1 - 3 files changed, 43 deletions(-) delete mode 100644 tests/unit/test_error_v_creation/test_error_html.html delete mode 100644 tests/unit/test_error_v_creation/test_error_pprint.txt delete mode 100644 tests/unit/test_error_v_creation/test_error_str.txt diff --git a/tests/unit/test_error_v_creation/test_error_html.html b/tests/unit/test_error_v_creation/test_error_html.html deleted file mode 100644 index 09d6855..0000000 --- a/tests/unit/test_error_v_creation/test_error_html.html +++ /dev/null @@ -1,41 +0,0 @@ -
- -
-
-
ErrorV
-
instance_index=n
- - - -
code0
message
-
-
-
diff --git a/tests/unit/test_error_v_creation/test_error_pprint.txt b/tests/unit/test_error_v_creation/test_error_pprint.txt deleted file mode 100644 index c74e0c2..0000000 --- a/tests/unit/test_error_v_creation/test_error_pprint.txt +++ /dev/null @@ -1 +0,0 @@ -ErrorV(instance_index=n, code=0, message=) diff --git a/tests/unit/test_error_v_creation/test_error_str.txt b/tests/unit/test_error_v_creation/test_error_str.txt deleted file mode 100644 index c74e0c2..0000000 --- a/tests/unit/test_error_v_creation/test_error_str.txt +++ /dev/null @@ -1 +0,0 @@ -ErrorV(instance_index=n, code=0, message=) From ed37fba0322a26726946206c52f3c9d644e25e30 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 13:55:46 +0200 Subject: [PATCH 34/49] Remove pytest-regressions dependency --- pyproject.toml | 1 - requirements-only-tests-locked.txt | 3 --- requirements-only-tests-min-locked.txt | 3 --- uv.lock | 36 +------------------------- 4 files changed, 1 insertion(+), 42 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index acdf664..e7ff79a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -102,7 +102,6 @@ tests-min = [ # Key dependencies # ---------------- "pytest>=8.3.4", - "pytest-regressions>=2.8.2", ] # Full test dependencies. tests-full = [ diff --git a/requirements-only-tests-locked.txt b/requirements-only-tests-locked.txt index 07d8697..1b70986 100644 --- a/requirements-only-tests-locked.txt +++ b/requirements-only-tests-locked.txt @@ -23,9 +23,6 @@ pure-eval==0.2.3 pygments==2.19.1 pytest==8.3.4 pytest-cov==6.0.0 -pytest-datadir==1.8.0 -pytest-regressions==2.8.2 -pyyaml==6.0.2 stack-data==0.6.3 tomli==2.2.1 ; python_full_version <= '3.11' traitlets==5.14.3 diff --git a/requirements-only-tests-min-locked.txt b/requirements-only-tests-min-locked.txt index 9b660cf..f0aaf57 100644 --- a/requirements-only-tests-min-locked.txt +++ b/requirements-only-tests-min-locked.txt @@ -6,8 +6,5 @@ iniconfig==2.0.0 packaging==24.2 pluggy==1.5.0 pytest==8.3.4 -pytest-datadir==1.8.0 -pytest-regressions==2.8.2 -pyyaml==6.0.2 tomli==2.2.1 ; python_full_version < '3.11' typing-extensions==4.12.2 ; python_full_version < '3.11' diff --git a/uv.lock b/uv.lock index 3a56b43..b156c3c 100644 --- a/uv.lock +++ b/uv.lock @@ -789,7 +789,6 @@ all-dev = [ { name = "pymdown-extensions" }, { name = "pytest" }, { name = "pytest-cov" }, - { name = "pytest-regressions" }, { name = "ruff" }, { name = "setuptools" }, { name = "tomli" }, @@ -835,14 +834,12 @@ tests = [ { name = "ipython", version = "9.4.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.11'" }, { name = "pytest" }, { name = "pytest-cov" }, - { name = "pytest-regressions" }, ] tests-full = [ { name = "pytest-cov" }, ] tests-min = [ { name = "pytest" }, - { name = "pytest-regressions" }, ] [package.metadata] @@ -882,7 +879,6 @@ all-dev = [ { name = "pymdown-extensions", specifier = ">=10.16.1" }, { name = "pytest", specifier = ">=8.3.4" }, { name = "pytest-cov", specifier = ">=6.0.0" }, - { name = "pytest-regressions", specifier = ">=2.8.2" }, { name = "ruff", specifier = ">=0.12.8" }, { name = "setuptools", specifier = ">=75.6.0" }, { name = "tomli", specifier = ">=2.2.1" }, @@ -926,13 +922,9 @@ tests = [ { name = "ipython", specifier = ">=8.18.1" }, { name = "pytest", specifier = ">=8.3.4" }, { name = "pytest-cov", specifier = ">=6.0.0" }, - { name = "pytest-regressions", specifier = ">=2.8.2" }, ] tests-full = [{ name = "pytest-cov", specifier = ">=6.0.0" }] -tests-min = [ - { name = "pytest", specifier = ">=8.3.4" }, - { name = "pytest-regressions", specifier = ">=2.8.2" }, -] +tests-min = [{ name = "pytest", specifier = ">=8.3.4" }] [[package]] name = "exceptiongroup" @@ -3019,32 +3011,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/36/3b/48e79f2cd6a61dbbd4807b4ed46cb564b4fd50a76166b1c4ea5c1d9e2371/pytest_cov-6.0.0-py3-none-any.whl", hash = "sha256:eee6f1b9e61008bd34975a4d5bab25801eb31898b032dd55addc93e96fcaaa35", size = 22949, upload-time = "2024-10-29T20:13:33.215Z" }, ] -[[package]] -name = "pytest-datadir" -version = "1.8.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "pytest" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/b4/46/db060b291999ca048edd06d6fa9ee95945d088edc38b1172c59eeb46ec45/pytest_datadir-1.8.0.tar.gz", hash = "sha256:7a15faed76cebe87cc91941dd1920a9a38eba56a09c11e9ddf1434d28a0f78eb", size = 11848, upload-time = "2025-07-30T13:52:12.518Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/8f/7a/33895863aec26ac3bb5068a73583f935680d6ab6af2a9567d409430c3ee1/pytest_datadir-1.8.0-py3-none-any.whl", hash = "sha256:5c677bc097d907ac71ca418109adc3abe34cf0bddfe6cf78aecfbabd96a15cf0", size = 6512, upload-time = "2025-07-30T13:52:11.525Z" }, -] - -[[package]] -name = "pytest-regressions" -version = "2.8.2" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "pytest" }, - { name = "pytest-datadir" }, - { name = "pyyaml" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/c1/8e/e17b12ee49a9814ad86f544225331406e3bf23849a5bc83f91e119e2b72b/pytest_regressions-2.8.2.tar.gz", hash = "sha256:1d8f4767be58b9994bfa7d60271099469ad32b8ca9f9d9ceca1c1d6827156b19", size = 116642, upload-time = "2025-08-13T15:50:37.961Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/b6/c9/2c3ea968876f698035b1f00dbe5cd6bff76d3e441dfb1af32111f544b6bb/pytest_regressions-2.8.2-py3-none-any.whl", hash = "sha256:a0804c1ce66d8e4d9a3c7c68f42a3d436182edca8e86565c232caeaf9e080fc2", size = 24856, upload-time = "2025-08-13T15:50:36.431Z" }, -] - [[package]] name = "python-dateutil" version = "2.9.0.post0" From 49708b5768a231a8579632244fa01535b5116b93 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 13:59:49 +0200 Subject: [PATCH 35/49] Remove ipython dependency --- pyproject.toml | 1 - requirements-only-tests-locked.txt | 20 +------------------- uv.lock | 8 -------- 3 files changed, 1 insertion(+), 28 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e7ff79a..72ea68f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -115,7 +115,6 @@ tests-full = [ tests = [ {include-group = "tests-min"}, {include-group = "tests-full"}, - "ipython>=8.18.1", ] all-dev = [ {include-group = "dev"}, diff --git a/requirements-only-tests-locked.txt b/requirements-only-tests-locked.txt index 1b70986..58f28a8 100644 --- a/requirements-only-tests-locked.txt +++ b/requirements-only-tests-locked.txt @@ -1,30 +1,12 @@ # This file was autogenerated by uv via the following command: # uv export -o requirements-only-tests-locked.txt --no-hashes --no-dev --no-emit-project --only-group tests -asttokens==3.0.0 colorama==0.4.6 ; sys_platform == 'win32' coverage==7.6.10 -decorator==5.1.1 exceptiongroup==1.3.0 ; python_full_version < '3.11' -executing==2.1.0 iniconfig==2.0.0 -ipython==8.18.1 ; python_full_version < '3.10' -ipython==8.37.0 ; python_full_version == '3.10.*' -ipython==9.4.0 ; python_full_version >= '3.11' -ipython-pygments-lexers==1.1.1 ; python_full_version >= '3.11' -jedi==0.19.2 -matplotlib-inline==0.1.7 packaging==24.2 -parso==0.8.4 -pexpect==4.9.0 ; (python_full_version < '3.10' and sys_platform == 'emscripten') or (sys_platform != 'emscripten' and sys_platform != 'win32') pluggy==1.5.0 -prompt-toolkit==3.0.48 -ptyprocess==0.7.0 ; (python_full_version < '3.10' and sys_platform == 'emscripten') or (sys_platform != 'emscripten' and sys_platform != 'win32') -pure-eval==0.2.3 -pygments==2.19.1 pytest==8.3.4 pytest-cov==6.0.0 -stack-data==0.6.3 tomli==2.2.1 ; python_full_version <= '3.11' -traitlets==5.14.3 -typing-extensions==4.12.2 ; python_full_version < '3.12' -wcwidth==0.2.13 +typing-extensions==4.12.2 ; python_full_version < '3.11' diff --git a/uv.lock b/uv.lock index b156c3c..c6813f1 100644 --- a/uv.lock +++ b/uv.lock @@ -767,9 +767,6 @@ all-dev = [ { name = "ford" }, { name = "fprettify" }, { name = "graphviz" }, - { name = "ipython", version = "8.18.1", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.10'" }, - { name = "ipython", version = "8.37.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version == '3.10.*'" }, - { name = "ipython", version = "9.4.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.11'" }, { name = "jupyterlab" }, { name = "jupytext" }, { name = "liccheck" }, @@ -829,9 +826,6 @@ docs = [ { name = "ruff" }, ] tests = [ - { name = "ipython", version = "8.18.1", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.10'" }, - { name = "ipython", version = "8.37.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version == '3.10.*'" }, - { name = "ipython", version = "9.4.0", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.11'" }, { name = "pytest" }, { name = "pytest-cov" }, ] @@ -859,7 +853,6 @@ all-dev = [ { name = "ford", specifier = ">=6.0.1" }, { name = "fprettify", specifier = ">=0.3.7" }, { name = "graphviz", specifier = ">=0.21" }, - { name = "ipython", specifier = ">=8.18.1" }, { name = "jupyterlab", specifier = ">=4.4.5" }, { name = "jupytext", specifier = ">=1.17.2" }, { name = "liccheck", specifier = ">=0.9.2" }, @@ -919,7 +912,6 @@ docs = [ { name = "ruff", specifier = ">=0.12.8" }, ] tests = [ - { name = "ipython", specifier = ">=8.18.1" }, { name = "pytest", specifier = ">=8.3.4" }, { name = "pytest-cov", specifier = ">=6.0.0" }, ] From 453564fd1b806d7ed114edbee922507a988cb55a Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 14:05:37 +0200 Subject: [PATCH 36/49] Remove TODO file --- TODO.md | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 TODO.md diff --git a/TODO.md b/TODO.md deleted file mode 100644 index 5728c19..0000000 --- a/TODO.md +++ /dev/null @@ -1,9 +0,0 @@ -- read through everything - - check docstrings - - check naming (if you want to propose a consistent naming convention, go for it) -- consider adding `m_error_v_w.build_instances` too, might be headache to pass arrays of things up and down correctly -- clean up -- final review (including deleting this file) -- merge -- move onto a basic result type with a subclass specifically for integers - (`ResultInt` or something, outline here: https://github.com/openscm/example-fgen-basic/tree/result-type) From 09a37a2c8d946cd5b129573c30f0313f75bf4c28 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 14:12:14 +0200 Subject: [PATCH 37/49] mypy --- src/example_fgen_basic/error_v/creation.py | 4 +--- src/example_fgen_basic/error_v/passing.py | 8 ++++---- src/example_fgen_basic/typing.py | 9 +++++---- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/example_fgen_basic/error_v/creation.py b/src/example_fgen_basic/error_v/creation.py index bcc67af..a0695d6 100644 --- a/src/example_fgen_basic/error_v/creation.py +++ b/src/example_fgen_basic/error_v/creation.py @@ -19,9 +19,7 @@ except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover raise CompiledExtensionNotFoundError("example_fgen_basic._lib.m_error_v_w") from exc try: - from example_fgen_basic._lib import ( # type: ignore - m_error_v_creation_w, - ) + from example_fgen_basic._lib import m_error_v_creation_w except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover raise CompiledExtensionNotFoundError( "example_fgen_basic._lib.m_error_v_creation_w" diff --git a/src/example_fgen_basic/error_v/passing.py b/src/example_fgen_basic/error_v/passing.py index bea4642..c48f77b 100644 --- a/src/example_fgen_basic/error_v/passing.py +++ b/src/example_fgen_basic/error_v/passing.py @@ -21,9 +21,7 @@ except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover raise CompiledExtensionNotFoundError("example_fgen_basic._lib.m_error_v_w") from exc try: - from example_fgen_basic._lib import ( # type: ignore - m_error_v_passing_w, - ) + from example_fgen_basic._lib import m_error_v_passing_w except (ModuleNotFoundError, ImportError) as exc: # pragma: no cover raise CompiledExtensionNotFoundError( "example_fgen_basic._lib.m_error_v_passing_w" @@ -84,7 +82,9 @@ def pass_errors(invs: tuple[ErrorV, ...]) -> NP_ARRAY_OF_BOOL: ) # Convert the result to boolean - res_raw = m_error_v_passing_w.pass_errors(instance_indexes, n=instance_indexes.size) + res_raw: NP_ARRAY_OF_INT = m_error_v_passing_w.pass_errors( + instance_indexes, n=instance_indexes.size + ) res = res_raw.astype(bool) # Tell Fortran to finalise the objects on the Fortran side diff --git a/src/example_fgen_basic/typing.py b/src/example_fgen_basic/typing.py index f71051d..1938d5e 100644 --- a/src/example_fgen_basic/typing.py +++ b/src/example_fgen_basic/typing.py @@ -7,22 +7,23 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from typing import Any, TypeAlias, Union + from typing import Union import numpy as np import numpy.typing as npt + from typing_extensions import Any, TypeAlias - NP_ARRAY_OF_INT: TypeAlias = npt.NDArray[np.integer] + NP_ARRAY_OF_INT: TypeAlias = npt.NDArray[np.integer[Any]] """ Type alias for an array of numpy int """ - NP_ARRAY_OF_FLOAT: TypeAlias = npt.NDArray[np.floating] + NP_ARRAY_OF_FLOAT: TypeAlias = npt.NDArray[np.floating[Any]] """ Type alias for an array of numpy floats """ - NP_FLOAT_OR_INT: TypeAlias = Union[np.floating, np.integer] + NP_FLOAT_OR_INT: TypeAlias = Union[np.floating[Any], np.integer[Any]] """ Type alias for a numpy float or int (not complex) """ From 4c84fbb55557205b37a04d871253cbc8d12845f8 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 16:52:00 +0200 Subject: [PATCH 38/49] Align GHA with main --- .github/workflows/release.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index bea4913..a30cfcb 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -1,6 +1,7 @@ name: Release on: + workflow_dispatch: push: tags: ['v*'] From 804f46d065900aaaaf790a901b0aae1e3bc86a7a Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 17:00:43 +0200 Subject: [PATCH 39/49] Fix macosx deployment target --- .github/workflows/release.yaml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index a30cfcb..ebaec2e 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -28,7 +28,6 @@ jobs: pip install -r {project}/requirements-only-tests-min-locked.txt CIBW_TEST_COMMAND: >- pytest {project}/tests - MACOSX_DEPLOYMENT_TARGET: "13.0" strategy: fail-fast: false matrix: @@ -57,6 +56,14 @@ jobs: # TODO: figure out whether we need/want to use other compilers too compiler: "gcc" version: "13" + - name: Set deployment target - MacOS 13.0 + if: ${{ matrix.os == 'macos-13' }} + run: | + echo "MACOSX_DEPLOYMENT_TARGET=13.0" >> $GITHUB_ENV + - name: Set deployment target - MacOS 14.0 + if: ${{ matrix.os == 'macos-14' }} + run: | + echo "MACOSX_DEPLOYMENT_TARGET=14.0" >> $GITHUB_ENV - name: Specify LLVM - windows-arm if: ${{ matrix.os == 'windows-11-arm' }} shell: pwsh From 24f5ad49215b46b1e9d808739fe8d3c64c9c29a0 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 19:42:54 +0200 Subject: [PATCH 40/49] Try workaround for windows --- .github/workflows/ci.yaml | 2 ++ tests/conftest.py | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 6f3ba2e..c6755e2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -125,6 +125,8 @@ jobs: uv-dependency-install-flags: "-v --reinstall-package example-fgen-basic -Ccompile-args='-v' --all-extras --group tests " - name: Run tests run: | + echo $CI + ls C:\ProgramData\mingw64\mingw64\bin COV_DIR=`uv run --no-sync python -c 'from pathlib import Path; import example_fgen_basic; print(Path(example_fgen_basic.__file__).parent)'` uv run --no-sync pytest -r a -v tests src --doctest-modules --doctest-report ndiff --cov=${COV_DIR} --cov-report=term-missing --cov-report=xml uv run --no-sync coverage report diff --git a/tests/conftest.py b/tests/conftest.py index a048f0f..024cd53 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,3 +3,9 @@ See https://docs.pytest.org/en/7.1.x/reference/fixtures.html#conftest-py-sharing-fixtures-across-multiple-files """ + +import os +import sys + +if os.environ["CI"] == "true" and sys.platform() == "win32": + os.add_dll_directory("C:\\ProgramData\\mingw64\\mingw64\\bin") From 86cf532499c0a3caf3fd4d788a68d25f878619d6 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 19:46:44 +0200 Subject: [PATCH 41/49] Fix default check for CI --- src/example_fgen_basic/fpyfgen/base_finalisable.f90 | 3 +++ .../fpyfgen/derived_type_manager_helpers.f90 | 2 +- tests/conftest.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/example_fgen_basic/fpyfgen/base_finalisable.f90 b/src/example_fgen_basic/fpyfgen/base_finalisable.f90 index a55df95..5e5d1c8 100644 --- a/src/example_fgen_basic/fpyfgen/base_finalisable.f90 +++ b/src/example_fgen_basic/fpyfgen/base_finalisable.f90 @@ -37,6 +37,9 @@ subroutine derived_type_finalise(self) !! Finalise the instance (i.e. free/deallocate) import :: BaseFinalisable + + implicit none + class(BaseFinalisable), intent(inout) :: self end subroutine derived_type_finalise diff --git a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 index 137c963..a17b9ae 100644 --- a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 +++ b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 @@ -33,7 +33,7 @@ subroutine get_derived_type_free_instance_number(instance_index, n_instances, in class(BaseFinalisable), dimension(n_instances), intent(inout) :: instance_array !! Array of instances - integer :: i = 1 + integer :: i ! Default if no available models are found instance_index = invalid_instance_index diff --git a/tests/conftest.py b/tests/conftest.py index 024cd53..6e1ae88 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,5 +7,5 @@ import os import sys -if os.environ["CI"] == "true" and sys.platform() == "win32": +if os.environ.get("CI", "false") == "true" and sys.platform() == "win32": os.add_dll_directory("C:\\ProgramData\\mingw64\\mingw64\\bin") From 6109eb68977671a9027512c1cb8c9caed472d6cf Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 19:47:28 +0200 Subject: [PATCH 42/49] Run pre-commit --- src/example_fgen_basic/error_v/creation.f90 | 2 +- src/example_fgen_basic/error_v/creation_wrapper.f90 | 2 +- src/example_fgen_basic/error_v/error_v.f90 | 4 ++-- src/example_fgen_basic/error_v/error_v_manager.f90 | 4 ++-- src/example_fgen_basic/error_v/error_v_wrapper.f90 | 2 +- src/example_fgen_basic/error_v/passing.f90 | 2 +- src/example_fgen_basic/error_v/passing_wrapper.f90 | 2 +- src/example_fgen_basic/fpyfgen/base_finalisable.f90 | 4 ++-- .../fpyfgen/derived_type_manager_helpers.f90 | 2 +- tests/unit/test_error_v_creation.f90 | 4 ++-- 10 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/example_fgen_basic/error_v/creation.f90 b/src/example_fgen_basic/error_v/creation.f90 index aa54681..97aed75 100644 --- a/src/example_fgen_basic/error_v/creation.f90 +++ b/src/example_fgen_basic/error_v/creation.f90 @@ -6,7 +6,7 @@ module m_error_v_creation use m_error_v, only: ErrorV, NO_ERROR_CODE - implicit none + implicit none (type, external) private public :: create_error, create_errors diff --git a/src/example_fgen_basic/error_v/creation_wrapper.f90 b/src/example_fgen_basic/error_v/creation_wrapper.f90 index 34b3619..fbddaae 100644 --- a/src/example_fgen_basic/error_v/creation_wrapper.f90 +++ b/src/example_fgen_basic/error_v/creation_wrapper.f90 @@ -17,7 +17,7 @@ module m_error_v_creation_w error_v_manager_set_instance_index_to => set_instance_index_to, & error_v_manager_ensure_instance_array_size_is_at_least => ensure_instance_array_size_is_at_least - implicit none + implicit none (type, external) private public :: create_error, create_errors diff --git a/src/example_fgen_basic/error_v/error_v.f90 b/src/example_fgen_basic/error_v/error_v.f90 index a05363f..c0876bd 100644 --- a/src/example_fgen_basic/error_v/error_v.f90 +++ b/src/example_fgen_basic/error_v/error_v.f90 @@ -8,7 +8,7 @@ !> with the convention that a code of 0 indicates no error. module m_error_v - implicit none + implicit none (type, external) private integer, parameter, public :: NO_ERROR_CODE = 0 @@ -34,7 +34,7 @@ module m_error_v private - procedure, public:: build, finalise + procedure, public :: build, finalise ! get_res sort of not needed (?) ! get_err sort of not needed (?) diff --git a/src/example_fgen_basic/error_v/error_v_manager.f90 b/src/example_fgen_basic/error_v/error_v_manager.f90 index 6956a92..693a50f 100644 --- a/src/example_fgen_basic/error_v/error_v_manager.f90 +++ b/src/example_fgen_basic/error_v/error_v_manager.f90 @@ -6,7 +6,7 @@ module m_error_v_manager use m_error_v, only: ErrorV - implicit none + implicit none (type, external) private type(ErrorV), dimension(:), allocatable :: instance_array @@ -143,7 +143,7 @@ subroutine ensure_instance_array_size_is_at_least(n) ! Race conditions ? instance_available = .true. - elseif (size(instance_available) < n) then + else if (size(instance_available) < n) then allocate(tmp_instances(n)) tmp_instances(1:size(instance_array)) = instance_array diff --git a/src/example_fgen_basic/error_v/error_v_wrapper.f90 b/src/example_fgen_basic/error_v/error_v_wrapper.f90 index 893a86d..7825cc9 100644 --- a/src/example_fgen_basic/error_v/error_v_wrapper.f90 +++ b/src/example_fgen_basic/error_v/error_v_wrapper.f90 @@ -14,7 +14,7 @@ module m_error_v_w error_v_manager_get_instance => get_instance, & error_v_manager_ensure_instance_array_size_is_at_least => ensure_instance_array_size_is_at_least - implicit none + implicit none (type, external) private public :: build_instance, finalise_instance, finalise_instances, & diff --git a/src/example_fgen_basic/error_v/passing.f90 b/src/example_fgen_basic/error_v/passing.f90 index eb44274..c274eb7 100644 --- a/src/example_fgen_basic/error_v/passing.f90 +++ b/src/example_fgen_basic/error_v/passing.f90 @@ -6,7 +6,7 @@ module m_error_v_passing use m_error_v, only: ErrorV, NO_ERROR_CODE - implicit none + implicit none (type, external) private public :: pass_error, pass_errors diff --git a/src/example_fgen_basic/error_v/passing_wrapper.f90 b/src/example_fgen_basic/error_v/passing_wrapper.f90 index 7c3c96a..7fd899b 100644 --- a/src/example_fgen_basic/error_v/passing_wrapper.f90 +++ b/src/example_fgen_basic/error_v/passing_wrapper.f90 @@ -18,7 +18,7 @@ module m_error_v_passing_w ! error_v_manager_set_instance_index_to => set_instance_index_to, & ! error_v_manager_ensure_instance_array_size_is_at_least => ensure_instance_array_size_is_at_least - implicit none + implicit none (type, external) private public :: pass_error, pass_errors diff --git a/src/example_fgen_basic/fpyfgen/base_finalisable.f90 b/src/example_fgen_basic/fpyfgen/base_finalisable.f90 index 5e5d1c8..617ecc0 100644 --- a/src/example_fgen_basic/fpyfgen/base_finalisable.f90 +++ b/src/example_fgen_basic/fpyfgen/base_finalisable.f90 @@ -4,7 +4,7 @@ !> across the Python-Fortran interface. module fpyfgen_base_finalisable - implicit none + implicit none (type, external) private integer, parameter, public :: INVALID_INSTANCE_INDEX = -1 @@ -38,7 +38,7 @@ subroutine derived_type_finalise(self) import :: BaseFinalisable - implicit none + implicit none (type, external) class(BaseFinalisable), intent(inout) :: self diff --git a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 index a17b9ae..9a4148c 100644 --- a/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 +++ b/src/example_fgen_basic/fpyfgen/derived_type_manager_helpers.f90 @@ -3,7 +3,7 @@ module fpyfgen_derived_type_manager_helpers use fpyfgen_base_finalisable, only: BaseFinalisable, invalid_instance_index - implicit none + implicit none (type, external) private public :: get_derived_type_free_instance_number, & diff --git a/tests/unit/test_error_v_creation.f90 b/tests/unit/test_error_v_creation.f90 index 7d0391e..b5b8d85 100644 --- a/tests/unit/test_error_v_creation.f90 +++ b/tests/unit/test_error_v_creation.f90 @@ -2,12 +2,12 @@ module test_error_v_creation ! How to print to stdout - use ISO_Fortran_env, only: stdout => OUTPUT_UNIT + use, intrinsic :: ISO_Fortran_env, only: stdout => OUTPUT_UNIT use testdrive, only: new_unittest, unittest_type, error_type, check use kind_parameters, only: dp - implicit none + implicit none (type, external) private public :: collect_error_v_creation_tests From 735c87259997fd38d0a85354bde81dc7c2671e53 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 19:48:05 +0200 Subject: [PATCH 43/49] Fix escaping --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c6755e2..2174a3a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -126,7 +126,7 @@ jobs: - name: Run tests run: | echo $CI - ls C:\ProgramData\mingw64\mingw64\bin + ls C:\\ProgramData\\mingw64\\mingw64\\bin COV_DIR=`uv run --no-sync python -c 'from pathlib import Path; import example_fgen_basic; print(Path(example_fgen_basic.__file__).parent)'` uv run --no-sync pytest -r a -v tests src --doctest-modules --doctest-report ndiff --cov=${COV_DIR} --cov-report=term-missing --cov-report=xml uv run --no-sync coverage report From 0ac144a274ae7d1f7e091382dcce7220690d43ae Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 19:53:08 +0200 Subject: [PATCH 44/49] Try agaim --- .github/workflows/ci.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2174a3a..1a09f7a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -126,7 +126,8 @@ jobs: - name: Run tests run: | echo $CI - ls C:\\ProgramData\\mingw64\\mingw64\\bin + ls C:/mingw64/bin + # ls C:\\ProgramData\\mingw64\\mingw64\\bin COV_DIR=`uv run --no-sync python -c 'from pathlib import Path; import example_fgen_basic; print(Path(example_fgen_basic.__file__).parent)'` uv run --no-sync pytest -r a -v tests src --doctest-modules --doctest-report ndiff --cov=${COV_DIR} --cov-report=term-missing --cov-report=xml uv run --no-sync coverage report From dc5dd66d182fe3d0add79a7df3f171eaeaab6b11 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 19:55:36 +0200 Subject: [PATCH 45/49] Fix typo --- tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 6e1ae88..188b646 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,5 +7,5 @@ import os import sys -if os.environ.get("CI", "false") == "true" and sys.platform() == "win32": - os.add_dll_directory("C:\\ProgramData\\mingw64\\mingw64\\bin") +if os.environ.get("CI", "false") == "true" and sys.platform == "win32": + os.add_dll_directory("C:\\mingw64\\bin") From efaf816019ed58021bcb80869561aeb94972f887 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 19:57:10 +0200 Subject: [PATCH 46/49] Remove helpers --- .github/workflows/ci.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1a09f7a..6f3ba2e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -125,9 +125,6 @@ jobs: uv-dependency-install-flags: "-v --reinstall-package example-fgen-basic -Ccompile-args='-v' --all-extras --group tests " - name: Run tests run: | - echo $CI - ls C:/mingw64/bin - # ls C:\\ProgramData\\mingw64\\mingw64\\bin COV_DIR=`uv run --no-sync python -c 'from pathlib import Path; import example_fgen_basic; print(Path(example_fgen_basic.__file__).parent)'` uv run --no-sync pytest -r a -v tests src --doctest-modules --doctest-report ndiff --cov=${COV_DIR} --cov-report=term-missing --cov-report=xml uv run --no-sync coverage report From 5722fdb2190c4cc219008371f2dd19405bf5c741 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 21:38:32 +0200 Subject: [PATCH 47/49] Add notes and allow env var configuration --- .github/workflows/ci.yaml | 6 ++++++ tests/conftest.py | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 6f3ba2e..26f7df2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -113,6 +113,12 @@ jobs: # TODO: figure out whether we need/want to use other compilers too compiler: "gcc" version: "13" + - name: Set DLL directory environment variable + # Used when running the tests, see `tests/conftest.py` for details + if: ${{ startsWith(matrix.os, 'windows') }} + run: | + echo "PYTHON_ADD_DLL_DIRECTORY=C:\\mingw64\\bin" >> $GITHUB_ENV + - uses: ./.github/actions/setup with: python-version: ${{ matrix.python-version }} diff --git a/tests/conftest.py b/tests/conftest.py index 188b646..902c989 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,16 @@ """ import os -import sys -if os.environ.get("CI", "false") == "true" and sys.platform == "win32": - os.add_dll_directory("C:\\mingw64\\bin") +if dll_directory_to_add := os.environ.get("PYTHON_ADD_DLL_DIRECTORY", None): + # Add the directory which has the libgfortran.dll file. + # + # A super deep dive into this is here: + # https://stackoverflow.com/a/78276248 + # The tl;dr is - mingw64's linker can be tricked by windows craziness + # into linking a dynamic library even when we wanted only static links, + # so if you want to avoid this, link with something else (e.g. the MS linker). + # (From what I know, this isn't an issue for the built wheels + # thanks to the cleverness of delvewheel) + os.add_dll_directory(dll_directory_to_add) + # os.add_dll_directory("C:\\mingw64\\bin") From 034023a841278fa2d507280be5f7211d1edb2f3c Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 21:39:05 +0200 Subject: [PATCH 48/49] CHANGELOG --- changelog/25.feature.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/25.feature.md diff --git a/changelog/25.feature.md b/changelog/25.feature.md new file mode 100644 index 0000000..ab137ca --- /dev/null +++ b/changelog/25.feature.md @@ -0,0 +1 @@ +Added demonstration of wrapping Fortran derived types From e63704f081a47ac6c17c216f35b47efa4cee06b8 Mon Sep 17 00:00:00 2001 From: Zebedee Nicholls Date: Fri, 5 Sep 2025 21:40:25 +0200 Subject: [PATCH 49/49] Omit *typing.py from code coverage --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 72ea68f..b9d8a0c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -148,6 +148,7 @@ source = [ ] branch = true omit = [ + "*typing.py", # Should not be needed at runtime # TODO: test these files directly when splitting out pyfgen_runtime "*exceptions.py", "*runtime_helpers.py",