Blocked PR on rational numbers exercise regarding parameters naming

Programming Fortran
1

The parameter’s names for rational numbers’s exercise function real_to_rational_pow were confusing since the base of the exponentiation was named ex while the exponent was name r1. They were renamed to base and ex_r1, respectively.

2

Do you have a link to the PR? The exercise? Is this specific to one track or the problem specs?

3

The PR link: changed real_to_rational_pow parameters names by guimolnar · Pull Request #193 · exercism/fortran · GitHub
This is specific to the following fortran exercise: Rational Numbers in Fortran on Exercism

This isn’t an issue per se, I just created this post because the automated answer from the github PR linked it as an alternative.

4

It looks like the Fortran track has a test generator script which, in theory, ought to be used to auto regenerate the tests as needed from the canonical data as needed.

Alas, when I try to use that, I get an exception:

Traceback (most recent call last):
  File "/home/goodi/play/codes/github/exercism/fortran/./bin/create_fortran_test.py", line 305, in <module>
    create_test(args.target, args.json)
  File "/home/goodi/play/codes/github/exercism/fortran/./bin/create_fortran_test.py", line 168, in create_test
    si.extend(create_single_test(j))
  File "/home/goodi/play/codes/github/exercism/fortran/./bin/create_fortran_test.py", line 124, in create_single_test
    si.extend(write_testcase(c, i))
  File "/home/goodi/play/codes/github/exercism/fortran/./bin/create_fortran_test.py", line 70, in write_testcase
    fcall = c['property']
KeyError: 'property'

I think the issue is that the test generator may not currently handle nested test cases. Rather than patching this one exercise manually, it might be more useful to fix the generator. Then exercises can be auto generated and not manually updated.

5

@ErikSchierboom It looks like pclausen may be the track maintainer but isn’t on the forums? Would you be interested in a PR to update the test generator? I have some code fixes that (1) better handles arbitrary depth nesting of test cases and (2) reads metadata from TOML in addition to YAML.

6

Here’s the code change I wrote up.

7

@IsaacG , I don’t fully understand your answer. This seems to be a script to generate a new <exercis>_test.f90 file, is that correct? All I changed was the functions parameters names, so it shouldn’t affect the already existing tests. In fact, when I ran

$ make
$ ctest -V

all tests have passed.

8

Ah. My bad. I confused the test file with the solution stub file. It does appear as though the stub files are hand generated and hand maintained, so much of what I said isn’t relevant to this PR. Sorry for the confusion.

The track currently indicates they are not presently accepting contributions so I’ll leave any further discussion about that change to the track maintainers to decide if they are interested in the proposed change.

9

Yep, that sounds good!

10

PR submitted