Result type int #36
Result type int #36
Conversation
Add first test of raising error on python side
| function build(self, data_v_in, error_v_in) result(res) | ||
| !! Build instance |
There was a problem hiding this comment.
| function build(self, data_v_in, error_v_in) result(res) | |
| !! Build instance | |
| subroutine build(self, data_v_in, error_v_in, res) | |
| !! Build instance |
I'm not sure what I like more. I don't love the pattern of using a function with effectively multiple return values. Now that I've seen it, using a subroutine seems to be a better option. What do you think?
| @define | ||
| class ResultInt: | ||
| """ | ||
| Result type that can hold double precision real values |
There was a problem hiding this comment.
| Result type that can hold double precision real values | |
| Result type that can hold (8 bit) integer values |
| call get_available_instance_index(res_available_instance_index) | ||
| ! MZ check for errors ? |
There was a problem hiding this comment.
yep we want to switch this to returniing ResultInt
| res_build = instance_array(instance_index) % build(data_v_in=data_v_in, error_v_in=error_v_in) | ||
| call get_available_instance_index(res_available_instance_index) | ||
| ! MZ check for errors ? | ||
| ! MZ function with side effect: good idea?? |
There was a problem hiding this comment.
ye I don't like it either, see my comments on the function
| call get_available_instance_index(res_available_instance_index) | ||
| ! MZ check for errors ? | ||
| ! MZ function with side effect: good idea?? | ||
| ! MZ why res_build is ResultNone?? |
There was a problem hiding this comment.
we're calling a method on the object we already have so I was following the python convention of returning none which means the result has to be ResultNone. Is there another option?
| ! print *, "Index ", instance_index, " has not been claimed" | ||
| ! error stop 1 | ||
| ! MZ Weird thing allocatable message | ||
| msg = "" |
| error stop 1 | ||
| ! print *, "Index ", instance_index, " has not been claimed" | ||
| ! error stop 1 | ||
| ! MZ Weird thing allocatable message |
There was a problem hiding this comment.
yep lots of mucking around. We may end up writing something like this https://gitlab.com/magicc/fgen/-/blob/switch-to-uv/packages/fgen-core/src/char_conversions.f90?ref_type=heads
or using someone else's equivalent if we can find a nice one
There was a problem hiding this comment.
ok the pros are here: https://github.com/fortran-lang/stdlib
we would either use their functions or could also just look at them and write our own versions because a) stdlib is MIT licensed so it's fine and b) we don't introduce the entire stdlib as a dependency that way
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #36 +/- ##
===========================================
- Coverage 100.00% 94.59% -5.41%
===========================================
Files 7 11 +4
Lines 68 148 +80
Branches 0 6 +6
===========================================
+ Hits 68 140 +72
- Misses 0 5 +5
- Partials 0 3 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Added
ResultIntand first attempt to bubble-up errorsChecklist
Please confirm that this pull request has done the following:
changelog/