Result type 1D integer #33

mzecc · 2025-09-09T16:50:58Z

Description

Providing classes to handle 1D integer arrays of results.

Checklist

Please confirm that this pull request has done the following:

Tests added
Documentation added (where applicable)
Changelog item added to changelog/

codecov · 2025-09-09T16:51:45Z

Codecov Report

❌ Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.52%. Comparing base (ca2a5f5) to head (4322463).
⚠️ Report is 1 commits behind head on cross-reference.

Files with missing lines	Patch %	Lines
src/example_fgen_basic/get_square_root.py	88.23%	1 Missing and 1 partial ⚠️
src/example_fgen_basic/result/result_dp.py	92.85%	2 Missing ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##           cross-reference      #33      +/-   ##
===================================================
- Coverage           100.00%   96.52%   -3.48%     
===================================================
  Files                    7       10       +3     
  Lines                   68      115      +47     
  Branches                 0        4       +4     
===================================================
+ Hits                    68      111      +43     
- Misses                   0        3       +3     
- Partials                 0        1       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

znicholls

Nice - going in the right direction

znicholls · 2025-09-10T08:09:22Z

src/example_fgen_basic/result/result.f90

+        ! class(*), allocatable :: data_v(..)
+        ! MZ: assumed rank can only be dummy argument NOT type/class argument
+        ! Data i.e. the result (if no error occurs)
+        !
+        ! Assumed rank array
+        ! (https://fortran-lang.discourse.group/t/assumed-rank-arrays/1049)
+        ! Technically a Fortran 2018 feature,
+        ! so maybe we need to update our file extensions.
+        ! If we can't use this, just comment this out
+        ! and leave each subclass of Result to set its data type
+        ! (e.g. ResultInteger will have `integer :: data`,
+        ! ResultDP1D will have `real(dp), dimension(:), allocatable :: data`)


Suggested change

! class(*), allocatable :: data_v(..)

! MZ: assumed rank can only be dummy argument NOT type/class argument

! Data i.e. the result (if no error occurs)

!

! Assumed rank array

! (https://fortran-lang.discourse.group/t/assumed-rank-arrays/1049)

! Technically a Fortran 2018 feature,

! so maybe we need to update our file extensions.

! If we can't use this, just comment this out

! and leave each subclass of Result to set its data type

! (e.g. ResultInteger will have `integer :: data`,

! ResultDP1D will have `real(dp), dimension(:), allocatable :: data`)

! Sub-classess hould have a `data` attribute

! We do not have that here, because Fortran does not have an `Any` type.

znicholls · 2025-09-10T08:09:46Z

src/example_fgen_basic/result/result.f90

+
+contains
+
+    subroutine clean_up(self)


Is clean_up the name to use for automatic deallocation ?

No it is not. The automatic deallocation is done using the final :: my_procedure.

But for abstract types I've found a problem:

Fortran requires that the dummy argument of a final procedure must be declared with the exact type, not the polymorphic class;

But an abstract type cannot be instantiated.

So final requires an exact type, but abstract types forbid exact-type dummies and that's a conflict I could not resolve.

The solution might be to have a clean_up procedure in the abstract type that gets called in the final routine of the child type to then make error_v to deallocate. I do not know if this makes sense.

super makes sense to me

znicholls · 2025-09-10T08:10:29Z

src/example_fgen_basic/result/result.f90

+        ! procedure, public:: build
+        ! TODO: Think about whether build should be on the abstract class
+        ! or just on each concrete implementation


Suggested change

! procedure, public:: build

! TODO: Think about whether build should be on the abstract class

! or just on each concrete implementation

! Sub-classes should implement a build method too.

! As above, we can't implement this here

! because Fortran doesn't have an `Any` type

! (which is what we would need for `data`)

znicholls · 2025-09-10T08:11:15Z

src/example_fgen_basic/result/result_int.f90

+    !!
+    !! Holds either an integer value or an error.
+
+        integer, allocatable :: data_v(:)


Suggested change

integer, allocatable :: data_v(:)

integer :: data_v

! or

integer, allocatable :: data_v

To start, let's just do an int, rather than a 1D array of int

I did an array because one has to take care of the allocation. But ok, no problem in doing scalars first.

src/example_fgen_basic/result/result_int.f90

znicholls · 2025-09-10T08:12:59Z

src/example_fgen_basic/result/result_int.f90

+
+    end function constructor
+
+    subroutine build(self, data_v_in, error_v_in)


Suggested change

subroutine build(self, data_v_in, error_v_in)

function build(self, data_v_in) result(error_v_in)

! or would the following work

function build(self, data_v_in) result(result_v)

! and we return a `ResultNone` type

znichollscr · 2025-09-10T09:54:23Z

src/example_fgen_basic/result/result_int.f90

+        ! Hopefully can leave without docstring (like Python)
+
+        if (allocated(self % data_v)) deallocate (self % data_v)
+        if (allocated(self % error_v)) call self % clean_up()


Suggested change

if (allocated(self % error_v)) call self % clean_up()

call self % clean_up()

Add first test of raising error on python side

znicholls and others added 8 commits August 31, 2025 13:16

Add notes

3c57801

Add ResultInt and clean up suggested Result implemenations

9402f91

class(results) fun

300694f

Playing around

844d29e

Result Integer 0D

89a8c79

Result Integer 0D

2f4ce95

Result-type : 1D integer

f23cf17

Merge branch 'main' into result-type

a5fba54

Fortitude CI

53b1910

znicholls changed the base branch from main to cross-reference September 10, 2025 08:07

znicholls reviewed Sep 10, 2025

View reviewed changes

znichollscr reviewed Sep 10, 2025

View reviewed changes

mzecc and others added 13 commits September 11, 2025 14:51

Result type

02b5392

Get fortran compiling

e2145a0

Add square root for easier illustration

7937334

Add failing tests

4b6f728

Up to writing wrapper

053fafd

Get test failing

61ba5dc

Get one test passing

9dc7b3f

Pass error raising test

31ebe4c

Skip test

0174330

Corrected create_errors small mistake

f60b408

Corrected error

999f35f

Mypy

3c2a6a4

Merge pull request #34 from openscm/result-type-zn

4322463

Add first test of raising error on python side

-        ! procedure, public:: build
-        ! TODO: Think about whether build should be on the abstract class
-        ! or just on each concrete implementation
+        ! Sub-classes should implement a build method too.
+        ! As above, we can't implement this here
+        ! because Fortran doesn't have an `Any` type
+        ! (which is what we would need for `data`)


		end function constructor

		subroutine build(self, data_v_in, error_v_in)

-    subroutine build(self, data_v_in, error_v_in)
+    function build(self, data_v_in) result(error_v_in)
+    ! or would the following work
+    function build(self, data_v_in) result(result_v)
+    ! and we return a `ResultNone` type

	if (allocated(self % error_v)) call self % clean_up()
	call self % clean_up()

Result type 1D integer #33

Are you sure you want to change the base?

Result type 1D integer #33

Uh oh!

Conversation

mzecc commented Sep 9, 2025

Description

Checklist

Uh oh!

codecov bot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

znicholls left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Sep 9, 2025 •

edited

Loading