search

hiker / fab

Flexible build system for scientific software
https://metomi.github.io/fab/
Other
1 stars 0 forks source link

Allowing file-specific transformation scripts #19

Closed jasonjunweilyu closed 3 months ago

jasonjunweilyu commented 4 months ago

This pull request is related to Issue #18. The purpose is to allow each x90 file to have its specific transformation script. Two files have changed:

  1. source/fab/steps/psyclone.py:

The psyclone function now takes in a function as the argument for the transformation_script parameter. This input function will be written by the user to take in an x90 file path (parameter fpath), and return a transformation script path.

  1. tests/system_tests/psyclone/test_psyclone_system_test.py:

Tests have been added to verify that the input transformation script function takes in the x90 file path and returns a transformation script path, which is then passed successfully to the psyclone command with -s. The checking for the transformation_script_hash is now done in the unit test, as now the transformation_script_hash is calculated when processing each file.

  1. tests/unit_tests/steps/test_psyclone_unit_test.py:

Transformation_script_hash test and prebuild_hash test have been updated.

jasonjunweilyu commented 4 months ago

Taken back the ready-for-review as I found there is a unit test needs to be fixed.

jasonjunweilyu commented 4 months ago

Now the mypy and flake8 issues are fixed. PSyclone unit test has been modified. This pull request is ready for review.

hiker commented 4 months ago

@jasonjunweilyu , sorry, I just realised I forgot two things: it would be good to have a real example of this function in the run configs (lfric/atm.py and lfric/gungho.py seems to use a script), so you would have to update them, and include the proper function to use (i.e. a function that does the current Makefile behaviour - i.e. checks for a file-specific script, if not global.py, if not None).

Also, surprisingly there is no documentation on how to use PSyclone at all (only the PSyclone overrides in doc/sources/advanced_config.rst). Can you add a section on how to use PSyclone todoc/source/writing_config.rst`? Just link to the PSyclone github page for details, but describe how a PSyclone step is added, including an example of a transformation script (i.e. the one you have added to the run_configs above).

jasonjunweilyu commented 4 months ago

Hi @hiker , I just finished making changes according to your comments. Now you can re-review my PR. Thanks.

jasonjunweilyu commented 4 months ago

Hi @hiker , I finished making modifications. Please review the changes when you have time. Thanks.

jasonjunweilyu commented 3 months ago

Hi @hiker , thanks a lot for the suggestions. It is not nit-picking. They all made the codes and docs better. I have modified accordingly.